[Qemu-devel] [PATCH] configure: show debug-info option in --help output

2013-03-24 Thread riegamaths
From: Dunrong Huang 

"--enable-debug-info" and "--disable-debug-info" were not shown
in --help output.

Signed-off-by: Dunrong Huang 
---
 configure | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure b/configure
index f2af714..61e71fa 100755
--- a/configure
+++ b/configure
@@ -1057,6 +1057,8 @@ echo "  --localstatedir=PATH install local state in 
PATH"
 echo "  --with-confsuffix=SUFFIX suffix for QEMU data inside datadir and 
sysconfdir [$confsuffix]"
 echo "  --enable-debug-tcg   enable TCG debugging"
 echo "  --disable-debug-tcg  disable TCG debugging (default)"
+echo "  --enable-debug-info   enable debugging information (default)"
+echo "  --disable-debug-info  disable debugging information"
 echo "  --enable-debug   enable common debug build options"
 echo "  --enable-sparse  enable sparse checker"
 echo "  --disable-sparse disable sparse checker (default)"
-- 
1.8.1.5




[Qemu-devel] [PATCH] block: Add options QDict to bdrv_file_open() prototypes (fix MinGW build)

2013-03-24 Thread Stefan Weil
The new parameter is unused yet.

This part was missing in commit 787e4a8500020695eb391e2f1cc4767ee071d441.

Cc: Kevin Wolf 
Cc: Eric Blake 
Signed-off-by: Stefan Weil 
---
 block/raw-win32.c |6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/raw-win32.c b/block/raw-win32.c
index 18e0068..ece2f1a 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -221,7 +221,8 @@ static void raw_parse_flags(int flags, int *access_flags, 
DWORD *overlapped)
 }
 }
 
-static int raw_open(BlockDriverState *bs, const char *filename, int flags)
+static int raw_open(BlockDriverState *bs, const char *filename,
+QDict *options, int flags)
 {
 BDRVRawState *s = bs->opaque;
 int access_flags;
@@ -494,7 +495,8 @@ static int hdev_probe_device(const char *filename)
 return 0;
 }
 
-static int hdev_open(BlockDriverState *bs, const char *filename, int flags)
+static int hdev_open(BlockDriverState *bs, const char *filename,
+ QDict *options, int flags)
 {
 BDRVRawState *s = bs->opaque;
 int access_flags, create_flags;
-- 
1.7.10.4




[Qemu-devel] [PATCH] rbd: fix compile error

2013-03-24 Thread Liu Yuan
From: Liu Yuan 

Commit 787e4a85 [block: Add options QDict to bdrv_file_open() prototypes] didn't
update rbd.c accordingly.

Cc: Kevin Wolf 
Cc: Stefan Hajnoczi 
Signed-off-by: Liu Yuan 
---
 block/rbd.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/rbd.c b/block/rbd.c
index 8cd10a7..1a8ea6d 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -441,7 +441,8 @@ static int qemu_rbd_aio_flush_cb(void *opaque)
 return (s->qemu_aio_count > 0);
 }
 
-static int qemu_rbd_open(BlockDriverState *bs, const char *filename, int flags)
+static int qemu_rbd_open(BlockDriverState *bs, const char *filename,
+ QDict *options, int flags)
 {
 BDRVRBDState *s = bs->opaque;
 char pool[RBD_MAX_POOL_NAME_SIZE];
-- 
1.7.9.5




[Qemu-devel] [PATCH] target-moxie: Fix pointer-to-integer conversion (MinGW-w64)

2013-03-24 Thread Stefan Weil
The type cast must use tcg_target_long instead of long.
This makes a difference for hosts where sizeof(long) != sizeof(void *).

Cc: Anthony Green 
Cc: Blue Swirl 
Signed-off-by: Stefan Weil 
---
 target-moxie/translate.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-moxie/translate.c b/target-moxie/translate.c
index 34f166e..cc02bd3 100644
--- a/target-moxie/translate.c
+++ b/target-moxie/translate.c
@@ -133,7 +133,7 @@ static inline void gen_goto_tb(CPUMoxieState *env, 
DisasContext *ctx,
 !ctx->singlestep_enabled) {
 tcg_gen_goto_tb(n);
 tcg_gen_movi_i32(cpu_pc, dest);
-tcg_gen_exit_tb((long) tb + n);
+tcg_gen_exit_tb((tcg_target_long)tb + n);
 } else {
 tcg_gen_movi_i32(cpu_pc, dest);
 if (ctx->singlestep_enabled) {
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH] rbd: fix compile error

2013-03-24 Thread Stefan Weil
Am 24.03.2013 08:41, schrieb Liu Yuan:
> From: Liu Yuan 
>
> Commit 787e4a85 [block: Add options QDict to bdrv_file_open() prototypes] 
> didn't
> update rbd.c accordingly.
>
> Cc: Kevin Wolf 
> Cc: Stefan Hajnoczi 
> Signed-off-by: Liu Yuan 
> ---
>  block/rbd.c |3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/block/rbd.c b/block/rbd.c
> index 8cd10a7..1a8ea6d 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -441,7 +441,8 @@ static int qemu_rbd_aio_flush_cb(void *opaque)
>  return (s->qemu_aio_count > 0);
>  }
>  
> -static int qemu_rbd_open(BlockDriverState *bs, const char *filename, int 
> flags)
> +static int qemu_rbd_open(BlockDriverState *bs, const char *filename,
> + QDict *options, int flags)
>  {
>  BDRVRBDState *s = bs->opaque;
>  char pool[RBD_MAX_POOL_NAME_SIZE];


Reviewed-by: Stefan Weil 




Re: [Qemu-devel] [PATCH] pcie: Enhance PCIe links

2013-03-24 Thread Michael S. Tsirkin
On Fri, Mar 22, 2013 at 09:25:13AM -0600, Alex Williamson wrote:
> On Thu, 2013-03-21 at 18:56 +0200, Michael S. Tsirkin wrote:
> > On Tue, Mar 19, 2013 at 04:24:47PM -0600, Alex Williamson wrote:
> > > Enable PCIe devices to negotiate links.  This upgrades our root ports
> > > and switches to advertising x16, 8.0GT/s and negotiates the current
> > > link status to the best available width and speed.  Note that we also
> > > skip setting link fields for Root Complex Integrated Endpoints as
> > > indicated by the PCIe spec.
> > > 
> > > Signed-off-by: Alex Williamson 
> > > ---
> > > 
> > > This depends on the previous pcie_endpoint_cap_init patch.
> > > 
> > >  hw/ioh3420.c|5 +-
> > >  hw/pci/pcie.c   |  150 
> > > ---
> > >  hw/pci/pcie.h   |7 ++
> > >  hw/pci/pcie_regs.h  |   17 +
> > >  hw/usb/hcd-xhci.c   |3 +
> > >  hw/xio3130_downstream.c |4 +
> > >  hw/xio3130_upstream.c   |4 +
> > >  7 files changed, 173 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/hw/ioh3420.c b/hw/ioh3420.c
> > > index 5cff61e..0aaec5b 100644
> > > --- a/hw/ioh3420.c
> > > +++ b/hw/ioh3420.c
> > > @@ -115,7 +115,10 @@ static int ioh3420_initfn(PCIDevice *d)
> > >  if (rc < 0) {
> > >  goto err_bridge;
> > >  }
> > > -rc = pcie_cap_init(d, IOH_EP_EXP_OFFSET, PCI_EXP_TYPE_ROOT_PORT, 
> > > p->port);
> > > +
> > > +/* Real hardware only supports up to x4, 2.5GT/s, but we're not real 
> > > hw */
> > > +rc = pcie_cap_init(d, IOH_EP_EXP_OFFSET, PCI_EXP_TYPE_ROOT_PORT, 
> > > p->port,
> > > +   PCI_EXP_LNK_MLW_16, PCI_EXP_LNK_LS_80);
> > >  if (rc < 0) {
> > >  goto err_msi;
> > >
> > I'd like to see some documentation about why this is needed/a good idea.
> > You could argue that some guest might be surprised if the card width
> > does not match reality but it could work in reverse too ...
> > Do you see some drivers complaining? Linux prints warnings sometimes -
> > is this what worries you?
> > Let's document the motivation here not only what's going on.
> 
> Ok, I can add motivation.  This is where I really wish we had a generic
> switch that wouldn't risk having existing real world expectations.  The
> base motivation though is to not create artificial bottlenecks.  If I
> assign a graphics card to a guest, I want the link to negotiate to the
> same bandwidth it has on the host.  I'm not entirely sure how to do that
> yet, whether I should cap the device capabilities to it's current status
> or whether I should force a negotiation at the current speed.  The
> former may confuse drivers if they expect certain device capabilities,
> the latter may cause drivers to attempt to renegotiate higher speeds.
> The goal though is to have a virtual platform that advertises sufficient
> speed on all ports to attach any real or virtual device.
> 
> Perhaps I should stick to hardware limitations for xio3130 & io3420 and
> distill these drivers down to generic ones with x32 ports and 8GT/s.

Hmm this doesn't actually answer the question ...  Why does it matter
what is the width negotiated by the guest?

> > > diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> > > index 62bd0b8..c07d3cc 100644
> > > --- a/hw/pci/pcie.c
> > > +++ b/hw/pci/pcie.c
> > > @@ -37,11 +37,98 @@
> > >  #define PCIE_DEV_PRINTF(dev, fmt, ...)  \
> > >  PCIE_DPRINTF("%s:%x "fmt, (dev)->name, (dev)->devfn, ## __VA_ARGS__)
> > >  
> > > +static uint16_t pcie_link_max_width(PCIDevice *dev)
> > > +{
> > > +uint8_t *exp_cap;
> > > +uint32_t lnkcap;
> > > +
> > > +exp_cap = dev->config + dev->exp.exp_cap;
> > > +lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP);
> > > +
> > > +return lnkcap & PCI_EXP_LNKCAP_MLW;
> > > +}
> > > +
> > > +static uint8_t pcie_link_speed_mask(PCIDevice *dev)
> > > +{
> > > +uint8_t *exp_cap;
> > > +uint32_t lnkcap, lnkcap2;
> > > +uint8_t speeds, mask;
> > > +
> > > +exp_cap = dev->config + dev->exp.exp_cap;
> > > +lnkcap = pci_get_long(exp_cap + PCI_EXP_LNKCAP);
> > > +lnkcap2 = pci_get_long(exp_cap + PCI_EXP_LNKCAP2);
> > > +
> > > +mask = (1 << (lnkcap & PCI_EXP_LNKCAP_SLS)) - 1;
> > > +
> > > +/*
> > > + * If LNKCAP2 reports supported link speeds, then LNKCAP indexes
> > > + * the highest supported speed.  Mask out the rest and return.
> > > + */
> > > +speeds = (lnkcap2 & 0xfe) >> 1;
> > 
> > what's 0xfe?
> 
> Will add macro
> 
> > > +if (speeds) {
> > > +return speeds & mask;
> > > +}
> > > +
> > > +/*
> > > + * Otherwise LNKCAP returns the maximum speed and the device supports
> > > + * all speeds below it.  This is really only valid for 2.5 & 5GT/s
> > > + */
> > > +return mask;
> > > +}
> > > +
> > > +void pcie_negotiate_link(PCIDevice *dev)
> > > +{
> > > +PCIDevice *parent;
> > > +uint16_t flags, width;
> > > +uint8_t type, spe

Re: [Qemu-devel] [PATCH] linux-user: improve target_to_host_sock_type conversion for MIPS

2013-03-24 Thread Aurelien Jarno
I am reviewing that for the MIPS part, and I am not really used to the
linux-user code, so a review from someone familiar with it would be
appreciated.

I added Richard Henderson and Blue Swirl in Cc: as alpha and sparc are 
also affected by this issue.

On Mon, Mar 18, 2013 at 11:47:05PM +0100, Petar Jovanovic wrote:
> From: Petar Jovanovic 
> 
> Previous implementation has failed to take into account different value of
> SOCK_NONBLOCK on target (MIPS) and host, and existence of SOCK_CLOEXEC.
> The same conversion has to be applied both for do_socket and do_socketpair,
> so the code has been isolated in a static inline function.
> It is defined for MIPS target only.
> 
> enum sock_type in linux-user/socket.h has been extended to include
> TARGET_SOCK_CLOEXEC and TARGET_SOCK_NONBLOCK, similar to definition in libc.
> The patch also includes necessary code style changes (tab to spaces) in the
> header file in the MIPS #ifdef block touched by the change.
> 
> Signed-off-by: Petar Jovanovic 
> ---
>  linux-user/socket.h  |  172 
> ++
>  linux-user/syscall.c |   45 -
>  2 files changed, 119 insertions(+), 98 deletions(-)
> 
> diff --git a/linux-user/socket.h b/linux-user/socket.h
> index 339cae5..e4dfe56 100644
> --- a/linux-user/socket.h
> +++ b/linux-user/socket.h
> @@ -1,91 +1,101 @@
>  
>  #if defined(TARGET_MIPS)
> - // MIPS special values for constants
> -
> - /*
> -  * For setsockopt(2)
> -  *
> -  * This defines are ABI conformant as far as Linux supports these ...
> -  */
> - #define TARGET_SOL_SOCKET  0x
> -
> - #define TARGET_SO_DEBUG0x0001  /* Record debugging information. 
>  */
> - #define TARGET_SO_REUSEADDR0x0004  /* Allow reuse of local 
> addresses.  */
> - #define TARGET_SO_KEEPALIVE0x0008  /* Keep connections alive and 
> send
> -   SIGPIPE when they die.  */
> - #define TARGET_SO_DONTROUTE0x0010  /* Don't do local routing.  */
> - #define TARGET_SO_BROADCAST0x0020  /* Allow transmission of
> -   broadcast messages.  */
> - #define TARGET_SO_LINGER   0x0080  /* Block on close of a reliable
> -   socket to transmit pending data.  */
> - #define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data 
> in-band.  */
> - #if 0
> - To add: #define TARGET_SO_REUSEPORT 0x0200 /* Allow local address 
> and port reuse.  */
> - #endif
> -
> - #define TARGET_SO_TYPE 0x1008  /* Compatible name for SO_STYLE. 
>  */
> - #define TARGET_SO_STYLESO_TYPE /* Synonym */
> - #define TARGET_SO_ERROR0x1007  /* get error status and clear */
> - #define TARGET_SO_SNDBUF   0x1001  /* Send buffer size. */
> - #define TARGET_SO_RCVBUF   0x1002  /* Receive buffer. */
> - #define TARGET_SO_SNDLOWAT 0x1003  /* send low-water mark */
> - #define TARGET_SO_RCVLOWAT 0x1004  /* receive low-water mark */
> - #define TARGET_SO_SNDTIMEO 0x1005  /* send timeout */
> - #define TARGET_SO_RCVTIMEO 0x1006  /* receive timeout */
> - #define TARGET_SO_ACCEPTCONN   0x1009
> -
> - /* linux-specific, might as well be the same as on i386 */
> - #define TARGET_SO_NO_CHECK 11
> - #define TARGET_SO_PRIORITY 12
> - #define TARGET_SO_BSDCOMPAT14
> +/* MIPS special values for constants */
> +
> +/*
> + * For setsockopt(2)
> + *
> + * This defines are ABI conformant as far as Linux supports these ...
> + */
> +#define TARGET_SOL_SOCKET  0x
> +
> +#define TARGET_SO_DEBUG0x0001  /* Record debugging information. 
> */
> +#define TARGET_SO_REUSEADDR0x0004  /* Allow reuse of local 
> addresses. */
> +#define TARGET_SO_KEEPALIVE0x0008  /* Keep connections alive and send
> +  SIGPIPE when they die. */
> +#define TARGET_SO_DONTROUTE0x0010  /* Don't do local routing. */
> +#define TARGET_SO_BROADCAST0x0020  /* Allow transmission of
> +  broadcast messages. */
> +#define TARGET_SO_LINGER   0x0080  /* Block on close of a reliable
> +* socket to transmit pending 
> data.
> +*/
> +#define TARGET_SO_OOBINLINE 0x0100 /* Receive out-of-band data 
> in-band.
> +*/
> +#if 0
> +/* To add: Allow local address and port reuse. */
> +#define TARGET_SO_REUSEPORT 0x0200
> +#endif
> +
> +#define TARGET_SO_TYPE 0x1008  /* Compatible name for SO_STYLE. 
> */
> +#define TARGET_SO_STYLESO_TYPE /* Synonym */
> +#define TARGET_SO_ERROR0x1007  /* get error status and clear */
> +#define TARGET_SO_SNDBUF   0x1001  /* Send

Re: [Qemu-devel] [PATCH] Remove device_tree.o from hw/moxie/Makefile.objs.

2013-03-24 Thread Aurelien Jarno
On Sun, Mar 24, 2013 at 12:43:38AM -0400, Anthony Green wrote:
> Here's a fix for the build problem identified by Aurelien Jano here:
> http://lists.gnu.org/archive/html/qemu-devel/2013-03/msg04177.html
> 
> I hadn't tested with FDT enabled recently.
> 
> Thanks!
> 
> AG
> 
> Signed-off-by: Anthony Green 
> ---
>  hw/moxie/Makefile.objs | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/hw/moxie/Makefile.objs b/hw/moxie/Makefile.objs
> index d0772d1..a5f1742 100644
> --- a/hw/moxie/Makefile.objs
> +++ b/hw/moxie/Makefile.objs
> @@ -1,6 +1,5 @@
>  # moxie boards
>  obj-y = serial.o mc146818rtc.o vga.o
> -obj-$(CONFIG_FDT) += device_tree.o
>  
>  obj-y := $(addprefix ../,$(obj-y))
>  obj-y += moxiesim.o
> -- 
> 1.8.1.4
> 

Thanks, applied.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



[Qemu-devel] qemu-x86_64 on i386 host: SIGSEGV

2013-03-24 Thread Michael Tokarev
$ ./x86_64-linux-user/qemu-x86_64 bash64
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation Fault

$ gdb x86_64-linux-user/qemu-x86_64
(gdb) ru bash64
Program received signal SIGSEGV, Segmentation fault.
disas_insn (s=s@entry=0xcf98, pc_start=18446744073699066880)
at target-i386/translate.c:4107
4107b = ldub_code(s->pc);
(gdb) p *s
$1 = {override = -1, prefix = 1484501952, aflag = 1, dflag = 1484503884,
  pc = 18446744073699066880, is_jmp = 0, cs_base = 0, pe = 1, code32 = 1,
  lma = 1, code64 = 1, rex_x = 0, rex_b = 0, ss32 = 1, cc_op = 0, addseg = 0,
  f_st = 0, vm86 = 0, cpl = 3, iopl = 0, tf = 0, singlestep_enabled = 0,
  jmp_opt = 1, mem_index = 0, flags = 4243635, tb = 0xf50e9f88,
  popl_esp_hack = 0, rip_offset = 0, cpuid_features = 126614521,
  cpuid_ext_features = -2139086847, cpuid_ext2_features = 563194873,
  cpuid_ext3_features = 101}

This is with current git.  Previous versions (tried 1.1 and 1.4)
segfaults in the same place too.

Some binaries works - for example, gzip (it prints "qemu: Unsupported syscall:
202" on the way which is a different issue).

Thanks,

/mjt



Re: [Qemu-devel] qemu-x86_64 on i386 host: SIGSEGV

2013-03-24 Thread Peter Maydell
On 24 March 2013 10:43, Michael Tokarev  wrote:
> $ ./x86_64-linux-user/qemu-x86_64 bash64
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
> Segmentation Fault

Are 64 bit linux-user guests on 32 bit hosts supposed to work?
I would expect them to be at best pretty unreliable.

> $ gdb x86_64-linux-user/qemu-x86_64
> (gdb) ru bash64
> Program received signal SIGSEGV, Segmentation fault.
> disas_insn (s=s@entry=0xcf98, pc_start=18446744073699066880)
> at target-i386/translate.c:4107
> 4107b = ldub_code(s->pc);
> (gdb) p *s
> $1 = {override = -1, prefix = 1484501952, aflag = 1, dflag = 1484503884,
>   pc = 18446744073699066880, is_jmp = 0, cs_base = 0, pe = 1, code32 = 1,

PC is FF600400 so either we've messed it up already or this
is just "64 bit address space doesn't fit in a 32 bit one".

> Some binaries works - for example, gzip (it prints "qemu: Unsupported syscall:
> 202" on the way which is a different issue).

Yes. That is just the usual "x86 linux-user isn't really supported":
202 is TARGET_NR_futex, which works on other target archs but
won't on x86 until somebody actually fixes support for threaded
guests in x86 to at least the level it is for other targets.

-- PMM



[Qemu-devel] [PATCH 02/10] versatile_pci: Expose PCI I/O region on Versatile PB

2013-03-24 Thread Peter Maydell
Comments in the QEMU source code claim that the version of the PCI
controller on the VersatilePB board doesn't support the PCI I/O
region, but this is incorrect; expose that region, map it in the
correct location, and drop the misleading comments.

This change removes the only currently implemented difference
between the realview-pci and versatile-pci models; however there
are other differences in not-yet-implemented functionality, so we
retain the distinction between the two device types.

Signed-off-by: Peter Maydell 
---
 hw/arm/versatilepb.c |3 +--
 hw/versatile_pci.c   |8 +++-
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index baaa265..0d08d15 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -226,14 +226,13 @@ static void versatile_init(QEMUMachineInitArgs *args, int 
board_id)
 qdev_init_nofail(dev);
 sysbus_mmio_map(busdev, 0, 0x4100); /* PCI self-config */
 sysbus_mmio_map(busdev, 1, 0x4200); /* PCI config */
+sysbus_mmio_map(busdev, 2, 0x4300); /* PCI I/O */
 sysbus_connect_irq(busdev, 0, sic[27]);
 sysbus_connect_irq(busdev, 1, sic[28]);
 sysbus_connect_irq(busdev, 2, sic[29]);
 sysbus_connect_irq(busdev, 3, sic[30]);
 pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci");
 
-/* The Versatile PCI bridge does not provide access to PCI IO space,
-   so many of the qemu PCI devices are not useable.  */
 for(n = 0; n < nb_nics; n++) {
 nd = &nd_table[n];
 
diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 16ce5d1..1312f46 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -77,7 +77,7 @@ static int pci_vpb_init(SysBusDevice *dev)
 /* Our memory regions are:
  * 0 : PCI self config window
  * 1 : PCI config window
- * 2 : PCI IO window (realview_pci only)
+ * 2 : PCI IO window
  */
 memory_region_init_io(&s->mem_config, &pci_vpb_config_ops, bus,
   "pci-vpb-selfconfig", 0x100);
@@ -85,10 +85,8 @@ static int pci_vpb_init(SysBusDevice *dev)
 memory_region_init_io(&s->mem_config2, &pci_vpb_config_ops, bus,
   "pci-vpb-config", 0x100);
 sysbus_init_mmio(dev, &s->mem_config2);
-if (s->realview) {
-isa_mmio_setup(&s->isa, 0x010);
-sysbus_init_mmio(dev, &s->isa);
-}
+isa_mmio_setup(&s->isa, 0x010);
+sysbus_init_mmio(dev, &s->isa);
 
 pci_create_simple(bus, -1, "versatile_pci_host");
 return 0;
-- 
1.7.9.5




[Qemu-devel] [PATCH 01/10] versatile_pci: Fix hardcoded tabs

2013-03-24 Thread Peter Maydell
There is just one line in this source file with a hardcoded tab
indent, so just fix it.

Signed-off-by: Peter Maydell 
---
 hw/versatile_pci.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 0b97a40..16ce5d1 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -104,7 +104,7 @@ static int pci_realview_init(SysBusDevice *dev)
 static int versatile_pci_host_init(PCIDevice *d)
 {
 pci_set_word(d->config + PCI_STATUS,
-PCI_STATUS_66MHZ | PCI_STATUS_DEVSEL_MEDIUM);
+ PCI_STATUS_66MHZ | PCI_STATUS_DEVSEL_MEDIUM);
 pci_set_byte(d->config + PCI_LATENCY_TIMER, 0x10);
 return 0;
 }
-- 
1.7.9.5




[Qemu-devel] [PATCH 08/10] versatile_pci: Implement the PCI controller's control registers

2013-03-24 Thread Peter Maydell
The versatile_pci PCI controller has a set of control registers which
handle the mapping between PCI and system address spaces. Implement
these registers (though for now they have no effect since we don't
implement mapping PCI space into system memory at all).

The most natural order for our sysbus regions has the control
registers at the start, so move all the others down one.

Signed-off-by: Peter Maydell 
---
 hw/arm/realview.c|7 +--
 hw/arm/versatilepb.c |7 +--
 hw/versatile_pci.c   |  135 --
 3 files changed, 138 insertions(+), 11 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 5fb490c..ba61d18 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -217,9 +217,10 @@ static void realview_init(QEMUMachineInitArgs *args,
 dev = qdev_create(NULL, "realview_pci");
 busdev = SYS_BUS_DEVICE(dev);
 qdev_init_nofail(dev);
-sysbus_mmio_map(busdev, 0, 0x6100); /* PCI self-config */
-sysbus_mmio_map(busdev, 1, 0x6200); /* PCI config */
-sysbus_mmio_map(busdev, 2, 0x6300); /* PCI I/O */
+sysbus_mmio_map(busdev, 0, 0x10019000); /* PCI controller registers */
+sysbus_mmio_map(busdev, 1, 0x6100); /* PCI self-config */
+sysbus_mmio_map(busdev, 2, 0x6200); /* PCI config */
+sysbus_mmio_map(busdev, 3, 0x6300); /* PCI I/O */
 sysbus_connect_irq(busdev, 0, pic[48]);
 sysbus_connect_irq(busdev, 1, pic[49]);
 sysbus_connect_irq(busdev, 2, pic[50]);
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 0d08d15..9c9bfde 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -224,9 +224,10 @@ static void versatile_init(QEMUMachineInitArgs *args, int 
board_id)
 dev = qdev_create(NULL, "versatile_pci");
 busdev = SYS_BUS_DEVICE(dev);
 qdev_init_nofail(dev);
-sysbus_mmio_map(busdev, 0, 0x4100); /* PCI self-config */
-sysbus_mmio_map(busdev, 1, 0x4200); /* PCI config */
-sysbus_mmio_map(busdev, 2, 0x4300); /* PCI I/O */
+sysbus_mmio_map(busdev, 0, 0x10001000); /* PCI controller regs */
+sysbus_mmio_map(busdev, 1, 0x4100); /* PCI self-config */
+sysbus_mmio_map(busdev, 2, 0x4200); /* PCI config */
+sysbus_mmio_map(busdev, 3, 0x4300); /* PCI I/O */
 sysbus_connect_irq(busdev, 0, sic[27]);
 sysbus_connect_irq(busdev, 1, sic[28]);
 sysbus_connect_irq(busdev, 2, sic[29]);
diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 7739f4c..a617675 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -17,6 +17,7 @@ typedef struct {
 PCIHostState parent_obj;
 
 qemu_irq irq[4];
+MemoryRegion controlregs;
 MemoryRegion mem_config;
 MemoryRegion mem_config2;
 MemoryRegion pci_io_space;
@@ -27,8 +28,27 @@ typedef struct {
 /* Constant for life of device: */
 int realview;
 uint8_t broken_irq_mapping;
+
+/* Register state: */
+uint32_t imap[3];
+uint32_t smap[3];
+uint32_t selfid;
+uint32_t flags;
 } PCIVPBState;
 
+static const VMStateDescription pci_vpb_vmstate = {
+.name = "versatile-pci",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32_ARRAY(imap, PCIVPBState, 3),
+VMSTATE_UINT32_ARRAY(smap, PCIVPBState, 3),
+VMSTATE_UINT32(selfid, PCIVPBState),
+VMSTATE_UINT32(flags, PCIVPBState),
+VMSTATE_END_OF_LIST()
+}
+};
+
 #define TYPE_VERSATILE_PCI "versatile_pci"
 #define PCI_VPB(obj) \
 OBJECT_CHECK(PCIVPBState, (obj), TYPE_VERSATILE_PCI)
@@ -37,6 +57,93 @@ typedef struct {
 #define PCI_VPB_HOST(obj) \
 OBJECT_CHECK(PCIDevice, (obj), TYPE_VERSATILE_PCIHOST)
 
+typedef enum {
+PCI_IMAP0 = 0x0,
+PCI_IMAP1 = 0x4,
+PCI_IMAP2 = 0x8,
+PCI_SELFID = 0xc,
+PCI_FLAGS = 0x10,
+PCI_SMAP0 = 0x14,
+PCI_SMAP1 = 0x18,
+PCI_SMAP2 = 0x1c,
+} PCIVPBControlRegs;
+
+static void pci_vpb_reg_write(void *opaque, hwaddr addr,
+  uint64_t val, unsigned size)
+{
+PCIVPBState *s = opaque;
+
+switch (addr) {
+case PCI_IMAP0:
+case PCI_IMAP1:
+case PCI_IMAP2:
+{
+int win = (addr - PCI_IMAP0) >> 2;
+s->imap[win] = val;
+break;
+}
+case PCI_SELFID:
+s->selfid = val;
+break;
+case PCI_FLAGS:
+s->flags = val;
+break;
+case PCI_SMAP0:
+case PCI_SMAP1:
+case PCI_SMAP2:
+{
+int win = (addr - PCI_SMAP0) >> 2;
+s->smap[win] = val;
+break;
+}
+default:
+qemu_log_mask(LOG_GUEST_ERROR,
+  "pci_vpb_reg_write: Bad offset %x\n", (int)addr);
+break;
+}
+}
+
+static uint64_t pci_vpb_reg_read(void *opaque, hwaddr addr,
+ unsigned size)
+{
+PCIVPBState *s = opaque;
+
+switch (addr) {
+case PCI_IMAP0:
+case PCI_IMAP1:
+case PCI_IMAP2:

[Qemu-devel] [PATCH 07/10] versatile_pci: Implement the correct PCI IRQ mapping

2013-03-24 Thread Peter Maydell
Implement the correct IRQ mapping for the Versatile PCI controller; it
differs between realview and versatile boards, but the previous QEMU
implementation was correct only for the first PCI card on a versatile
board, since we weren't swizzling IRQs based on the slot number.

Note that this change will break any uses of PCI on Linux kernels which
have an equivalent bug (since they have effectively only been tested
against QEMU, not real hardware). Unfortunately this currently means
"all Linux kernels" and "all uses of versatilepb with a hard disk"
since we default to a PCI SCSI controller.

We therefore provide a property for enabling the old broken IRQ mapping;
this can be enabled with the command line option:
  -global versatile_pci.broken-irq-mapping=1

Signed-off-by: Peter Maydell 
---
 hw/versatile_pci.c |   61 ++--
 1 file changed, 59 insertions(+), 2 deletions(-)

diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 576e619..7739f4c 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -26,6 +26,7 @@ typedef struct {
 
 /* Constant for life of device: */
 int realview;
+uint8_t broken_irq_mapping;
 } PCIVPBState;
 
 #define TYPE_VERSATILE_PCI "versatile_pci"
@@ -61,11 +62,52 @@ static const MemoryRegionOps pci_vpb_config_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int pci_vpb_map_irq(PCIDevice *d, int irq_num)
+static int pci_vpb_broken_map_irq(PCIDevice *d, int irq_num)
 {
+/* Map IRQs as old and buggy versions of QEMU have done in the past;
+ * this is not how hardware behaves, and it will not work with guests
+ * which drive the hardware correctly, but it allows us to work with
+ * buggy Linux kernels which were written against the buggy QEMU.
+ */
 return irq_num;
 }
 
+static int pci_vpb_map_irq(PCIDevice *d, int irq_num)
+{
+/* Slot to IRQ mapping for RealView Platform Baseboard 926 backplane
+ *  nameslotIntAIntBIntCIntD
+ *  A   31  IRQ28   IRQ29   IRQ30   IRQ27
+ *  B   30  IRQ27   IRQ28   IRQ29   IRQ30
+ *  C   29  IRQ30   IRQ27   IRQ28   IRQ29
+ * Slot C is for the host bridge; A and B the peripherals.
+ * Our output irqs 0..3 correspond to the baseboard's 27..30.
+ *
+ * This mapping function takes account of an oddity in the PB926
+ * board wiring, where the FPGA's P_nINTA input is connected to
+ * the INTB connection on the board PCI edge connector, P_nINTB
+ * is connected to INTC, and so on, so everything is one number
+ * further round from where you might expect.
+ */
+return (PCI_SLOT(d->devfn) + irq_num - 2) % PCI_NUM_PINS;
+}
+
+static int pci_vpb_rv_map_irq(PCIDevice *d, int irq_num)
+{
+/* Slot to IRQ mapping for RealView EB and PB1176 backplane
+ *  nameslotIntAIntBIntCIntD
+ *  A   31  IRQ50   IRQ51   IRQ48   IRQ49
+ *  B   30  IRQ49   IRQ50   IRQ51   IRQ48
+ *  C   29  IRQ48   IRQ49   IRQ50   IRQ51
+ * Slot C is for the host bridge; A and B the peripherals.
+ * Our output irqs 0..3 correspond to the baseboard's 48..51.
+ *
+ * The PB1176 and EB boards don't have the PB926 wiring oddity
+ * described above; P_nINTA connects to INTA, P_nINTB to INTB
+ * and so on, which is why this mapping function is different.
+ */
+return (PCI_SLOT(d->devfn) + irq_num - 1) % PCI_NUM_PINS;
+}
+
 static void pci_vpb_set_irq(void *opaque, int irq_num, int level)
 {
 qemu_irq *pic = opaque;
@@ -95,13 +137,22 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
 {
 PCIVPBState *s = PCI_VPB(dev);
 SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+pci_map_irq_fn mapfn;
 int i;
 
 for (i = 0; i < 4; i++) {
 sysbus_init_irq(sbd, &s->irq[i]);
 }
 
-pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, pci_vpb_map_irq, s->irq, 4);
+if (s->broken_irq_mapping) {
+mapfn = pci_vpb_broken_map_irq;
+} else if (s->realview) {
+mapfn = pci_vpb_rv_map_irq;
+} else {
+mapfn = pci_vpb_map_irq;
+}
+
+pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, mapfn, s->irq, 4);
 
 /* ??? Register memory space.  */
 
@@ -154,11 +205,17 @@ static const TypeInfo versatile_pci_host_info = {
 .class_init= versatile_pci_host_class_init,
 };
 
+static Property pci_vpb_properties[] = {
+DEFINE_PROP_UINT8("broken-irq-mapping", PCIVPBState, broken_irq_mapping, 
0),
+DEFINE_PROP_END_OF_LIST()
+};
+
 static void pci_vpb_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 
 dc->realize = pci_vpb_realize;
+dc->props = pci_vpb_properties;
 }
 
 static const TypeInfo pci_vpb_info = {
-- 
1.7.9.5




[Qemu-devel] [PATCH 06/10] versatile_pci: Put the host bridge PCI device at slot 29

2013-03-24 Thread Peter Maydell
On real hardware the host bridge appears as a PCI device in slot 29,
so make QEMU put its host bridge in that slot too.

Signed-off-by: Peter Maydell 
---
 hw/versatile_pci.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 777e9b1..576e619 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -87,6 +87,8 @@ static void pci_vpb_init(Object *obj)
 
 object_initialize(&s->pci_dev, TYPE_VERSATILE_PCI_HOST);
 qdev_set_parent_bus(DEVICE(&s->pci_dev), BUS(&s->pci_bus));
+object_property_set_int(OBJECT(&s->pci_dev), PCI_DEVFN(29, 0), "addr",
+NULL);
 }
 
 static void pci_vpb_realize(DeviceState *dev, Error **errp)
-- 
1.7.9.5




[Qemu-devel] [PATCH 03/10] versatile_pci: Update to realize and instance init functions

2013-03-24 Thread Peter Maydell
Update the Versatile PCI controller to use a realize function rather
than SysBusDevice::init. To reflect the fact that the 'realview_pci'
class is taking most of its implementation from 'versatile_pci' (and
to make the QOM casts work) we make 'realview_pci' a subclass of
'versatile_pci'.

Signed-off-by: Peter Maydell 
---
 hw/versatile_pci.c |   50 +-
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 1312f46..541f6b5 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -21,6 +21,14 @@ typedef struct {
 MemoryRegion isa;
 } PCIVPBState;
 
+#define TYPE_VERSATILE_PCI "versatile_pci"
+#define PCI_VPB(obj) \
+OBJECT_CHECK(PCIVPBState, (obj), TYPE_VERSATILE_PCI)
+
+#define TYPE_VERSATILE_PCI_HOST "versatile_pci_host"
+#define PCI_VPB_HOST(obj) \
+OBJECT_CHECK(PCIDevice, (obj), TYPE_VERSATILE_PCIHOST)
+
 static inline uint32_t vpb_pci_config_addr(hwaddr addr)
 {
 return addr & 0xff;
@@ -58,16 +66,17 @@ static void pci_vpb_set_irq(void *opaque, int irq_num, int 
level)
 qemu_set_irq(pic[irq_num], level);
 }
 
-static int pci_vpb_init(SysBusDevice *dev)
+static void pci_vpb_realize(DeviceState *dev, Error **errp)
 {
-PCIVPBState *s = FROM_SYSBUS(PCIVPBState, dev);
+PCIVPBState *s = PCI_VPB(dev);
+SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
 PCIBus *bus;
 int i;
 
 for (i = 0; i < 4; i++) {
-sysbus_init_irq(dev, &s->irq[i]);
+sysbus_init_irq(sbd, &s->irq[i]);
 }
-bus = pci_register_bus(&dev->qdev, "pci",
+bus = pci_register_bus(dev, "pci",
pci_vpb_set_irq, pci_vpb_map_irq, s->irq,
get_system_memory(), get_system_io(),
PCI_DEVFN(11, 0), 4);
@@ -81,22 +90,14 @@ static int pci_vpb_init(SysBusDevice *dev)
  */
 memory_region_init_io(&s->mem_config, &pci_vpb_config_ops, bus,
   "pci-vpb-selfconfig", 0x100);
-sysbus_init_mmio(dev, &s->mem_config);
+sysbus_init_mmio(sbd, &s->mem_config);
 memory_region_init_io(&s->mem_config2, &pci_vpb_config_ops, bus,
   "pci-vpb-config", 0x100);
-sysbus_init_mmio(dev, &s->mem_config2);
+sysbus_init_mmio(sbd, &s->mem_config2);
 isa_mmio_setup(&s->isa, 0x010);
-sysbus_init_mmio(dev, &s->isa);
+sysbus_init_mmio(sbd, &s->isa);
 
 pci_create_simple(bus, -1, "versatile_pci_host");
-return 0;
-}
-
-static int pci_realview_init(SysBusDevice *dev)
-{
-PCIVPBState *s = FROM_SYSBUS(PCIVPBState, dev);
-s->realview = 1;
-return pci_vpb_init(dev);
 }
 
 static int versatile_pci_host_init(PCIDevice *d)
@@ -118,7 +119,7 @@ static void versatile_pci_host_class_init(ObjectClass 
*klass, void *data)
 }
 
 static const TypeInfo versatile_pci_host_info = {
-.name  = "versatile_pci_host",
+.name  = TYPE_VERSATILE_PCI_HOST,
 .parent= TYPE_PCI_DEVICE,
 .instance_size = sizeof(PCIDevice),
 .class_init= versatile_pci_host_class_init,
@@ -126,30 +127,29 @@ static const TypeInfo versatile_pci_host_info = {
 
 static void pci_vpb_class_init(ObjectClass *klass, void *data)
 {
-SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
+DeviceClass *dc = DEVICE_CLASS(klass);
 
-sdc->init = pci_vpb_init;
+dc->realize = pci_vpb_realize;
 }
 
 static const TypeInfo pci_vpb_info = {
-.name  = "versatile_pci",
+.name  = TYPE_VERSATILE_PCI,
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(PCIVPBState),
 .class_init= pci_vpb_class_init,
 };
 
-static void pci_realview_class_init(ObjectClass *klass, void *data)
+static void pci_realview_init(Object *obj)
 {
-SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
+PCIVPBState *s = PCI_VPB(obj);
 
-sdc->init = pci_realview_init;
+s->realview = 1;
 }
 
 static const TypeInfo pci_realview_info = {
 .name  = "realview_pci",
-.parent= TYPE_SYS_BUS_DEVICE,
-.instance_size = sizeof(PCIVPBState),
-.class_init= pci_realview_class_init,
+.parent= TYPE_VERSATILE_PCI,
+.instance_init = pci_realview_init,
 };
 
 static void versatile_pci_register_types(void)
-- 
1.7.9.5




[Qemu-devel] [PATCH 05/10] versatile_pci: Use separate PCI I/O space rather than system I/O space

2013-03-24 Thread Peter Maydell
Rather than overloading the system I/O space (which doesn't even make
any sense on ARM) for PCI I/O, create an memory region in the PCI
controller and use that to represent the I/O space.

Signed-off-by: Peter Maydell 
---
 hw/versatile_pci.c |   17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index dfd3001..777e9b1 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -19,7 +19,8 @@ typedef struct {
 qemu_irq irq[4];
 MemoryRegion mem_config;
 MemoryRegion mem_config2;
-MemoryRegion isa;
+MemoryRegion pci_io_space;
+MemoryRegion pci_io_window;
 PCIBus pci_bus;
 PCIDevice pci_dev;
 
@@ -77,8 +78,10 @@ static void pci_vpb_init(Object *obj)
 PCIHostState *h = PCI_HOST_BRIDGE(obj);
 PCIVPBState *s = PCI_VPB(obj);
 
+memory_region_init(&s->pci_io_space, "pci_io", 1ULL << 32);
+
 pci_bus_new_inplace(&s->pci_bus, DEVICE(obj), "pci",
-get_system_memory(), get_system_io(),
+get_system_memory(), &s->pci_io_space,
 PCI_DEVFN(11, 0));
 h->bus = &s->pci_bus;
 
@@ -111,8 +114,14 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
 memory_region_init_io(&s->mem_config2, &pci_vpb_config_ops, &s->pci_bus,
   "pci-vpb-config", 0x100);
 sysbus_init_mmio(sbd, &s->mem_config2);
-isa_mmio_setup(&s->isa, 0x010);
-sysbus_init_mmio(sbd, &s->isa);
+
+/* The window into I/O space is always into a fixed base address;
+ * its size is the same for both realview and versatile.
+ */
+memory_region_init_alias(&s->pci_io_window, "pci-vbp-io-window",
+ &s->pci_io_space, 0, 0x10);
+
+sysbus_init_mmio(sbd, &s->pci_io_space);
 
 /* TODO Remove once realize propagates to child devices. */
 object_property_set_bool(OBJECT(&s->pci_dev), true, "realized", errp);
-- 
1.7.9.5




[Qemu-devel] [PATCH 09/10] arm/realview: Fix mapping of PCI regions

2013-03-24 Thread Peter Maydell
Fix the mapping of the PCI regions for the realview board, which were
all incorrect. (This was never noticed because the Linux kernel
doesn't actually include a PCI driver for the realview boards.)

Signed-off-by: Peter Maydell 
---
 hw/arm/realview.c |   18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index ba61d18..23c968b 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -218,9 +218,9 @@ static void realview_init(QEMUMachineInitArgs *args,
 busdev = SYS_BUS_DEVICE(dev);
 qdev_init_nofail(dev);
 sysbus_mmio_map(busdev, 0, 0x10019000); /* PCI controller registers */
-sysbus_mmio_map(busdev, 1, 0x6100); /* PCI self-config */
-sysbus_mmio_map(busdev, 2, 0x6200); /* PCI config */
-sysbus_mmio_map(busdev, 3, 0x6300); /* PCI I/O */
+sysbus_mmio_map(busdev, 1, 0x6000); /* PCI self-config */
+sysbus_mmio_map(busdev, 2, 0x6100); /* PCI config */
+sysbus_mmio_map(busdev, 3, 0x6200); /* PCI I/O */
 sysbus_connect_irq(busdev, 0, pic[48]);
 sysbus_connect_irq(busdev, 1, pic[49]);
 sysbus_connect_irq(busdev, 2, pic[50]);
@@ -304,12 +304,12 @@ static void realview_init(QEMUMachineInitArgs *args,
 /*  0x5800 PISMO.  */
 /*  0x5c00 PISMO.  */
 /* 0x6000 PCI.  */
-/* 0x6100 PCI Self Config.  */
-/* 0x6200 PCI Config.  */
-/* 0x6300 PCI IO.  */
-/* 0x6400 PCI mem 0.  */
-/* 0x6800 PCI mem 1.  */
-/* 0x6c00 PCI mem 2.  */
+/* 0x6000 PCI Self Config.  */
+/* 0x6100 PCI Config.  */
+/* 0x6200 PCI IO.  */
+/* 0x6300 PCI mem 0.  */
+/* 0x6400 PCI mem 1.  */
+/* 0x6800 PCI mem 2.  */
 
 /* ??? Hack to map an additional page of ram for the secondary CPU
startup code.  I guess this works on real hardware because the
-- 
1.7.9.5




[Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Peter Maydell
This patch series fixes a number of serious bugs in our emulation of
the PCI controller found on VersatilePB and the early Realview boards:
 * our interrupt mapping was totally wrong
 * we weren't implementing the PCI memory windows
 * the I/O window wasn't mapped on VersatilePB
 * realview mapped things at the wrong addresses
 * we didn't implement the controller's registers at all
It also updates to some reasonable approximation to QOM best practice,
including subclassing pci_host rather than doing everything by hand.

I haven't implemented support for the SMAP registers (which control
how the controller converts accesses made by bus-mastering PCI
devices into system addresses). For the moment we rely on the fact
that Linux always maps things 1:1. (It wouldn't be too hard to add
SMAP support but it requires changing QEMU's pci code to allow a
controller to pass in a MemoryRegion* for DMA to use instead of
the system address space, so I prefer to leave that for another series.)

Patchset tested on both versatilepb and realview, using a set of
Linux kernel patches written by Arnd Bergmann:
http://lists.infradead.org/pipermail/linux-arm-kernel/2010-October/029040.html
which were in turn tested against real PB926 and PB1176 hardware.


 * WARNING WARNING *

This patchset will break any use of PCI (including the default SCSI
card) on versatilepb with current Linux kernels, because those kernels
have the matching bug in interrupt mapping to old QEMU.

I've provided a property for enabling the old broken IRQ mapping;
this can be enabled with the command line option:
  -global versatile_pci.broken-irq-mapping=1

(If anybody wants to suggest a better way of handling this please do.)



Peter Maydell (10):
  versatile_pci: Fix hardcoded tabs
  versatile_pci: Expose PCI I/O region on Versatile PB
  versatile_pci: Update to realize and instance init functions
  versatile_pci: Change to subclassing TYPE_PCI_HOST_BRIDGE
  versatile_pci: Use separate PCI I/O space rather than system I/O space
  versatile_pci: Put the host bridge PCI device at slot 29
  versatile_pci: Implement the correct PCI IRQ mapping
  versatile_pci: Implement the PCI controller's control registers
  arm/realview: Fix mapping of PCI regions
  versatile_pci: Expose PCI memory space to system

 hw/arm/realview.c|   22 +--
 hw/arm/versatilepb.c |   11 +-
 hw/versatile_pci.c   |  368 --
 3 files changed, 344 insertions(+), 57 deletions(-)

-- 
1.7.9.5




[Qemu-devel] [PATCH 04/10] versatile_pci: Change to subclassing TYPE_PCI_HOST_BRIDGE

2013-03-24 Thread Peter Maydell
Change versatile_pci to subclass TYPE_PCI_HOST_BRIDGE and generally
handle PCI in a more QOM-like fashion.

Signed-off-by: Peter Maydell 
---
 hw/versatile_pci.c |   41 ++---
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index 541f6b5..dfd3001 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -9,16 +9,22 @@
 
 #include "hw/sysbus.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/pci_bus.h"
 #include "hw/pci/pci_host.h"
 #include "exec/address-spaces.h"
 
 typedef struct {
-SysBusDevice busdev;
+PCIHostState parent_obj;
+
 qemu_irq irq[4];
-int realview;
 MemoryRegion mem_config;
 MemoryRegion mem_config2;
 MemoryRegion isa;
+PCIBus pci_bus;
+PCIDevice pci_dev;
+
+/* Constant for life of device: */
+int realview;
 } PCIVPBState;
 
 #define TYPE_VERSATILE_PCI "versatile_pci"
@@ -66,20 +72,31 @@ static void pci_vpb_set_irq(void *opaque, int irq_num, int 
level)
 qemu_set_irq(pic[irq_num], level);
 }
 
+static void pci_vpb_init(Object *obj)
+{
+PCIHostState *h = PCI_HOST_BRIDGE(obj);
+PCIVPBState *s = PCI_VPB(obj);
+
+pci_bus_new_inplace(&s->pci_bus, DEVICE(obj), "pci",
+get_system_memory(), get_system_io(),
+PCI_DEVFN(11, 0));
+h->bus = &s->pci_bus;
+
+object_initialize(&s->pci_dev, TYPE_VERSATILE_PCI_HOST);
+qdev_set_parent_bus(DEVICE(&s->pci_dev), BUS(&s->pci_bus));
+}
+
 static void pci_vpb_realize(DeviceState *dev, Error **errp)
 {
 PCIVPBState *s = PCI_VPB(dev);
 SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
-PCIBus *bus;
 int i;
 
 for (i = 0; i < 4; i++) {
 sysbus_init_irq(sbd, &s->irq[i]);
 }
-bus = pci_register_bus(dev, "pci",
-   pci_vpb_set_irq, pci_vpb_map_irq, s->irq,
-   get_system_memory(), get_system_io(),
-   PCI_DEVFN(11, 0), 4);
+
+pci_bus_irqs(&s->pci_bus, pci_vpb_set_irq, pci_vpb_map_irq, s->irq, 4);
 
 /* ??? Register memory space.  */
 
@@ -88,16 +105,17 @@ static void pci_vpb_realize(DeviceState *dev, Error **errp)
  * 1 : PCI config window
  * 2 : PCI IO window
  */
-memory_region_init_io(&s->mem_config, &pci_vpb_config_ops, bus,
+memory_region_init_io(&s->mem_config, &pci_vpb_config_ops, &s->pci_bus,
   "pci-vpb-selfconfig", 0x100);
 sysbus_init_mmio(sbd, &s->mem_config);
-memory_region_init_io(&s->mem_config2, &pci_vpb_config_ops, bus,
+memory_region_init_io(&s->mem_config2, &pci_vpb_config_ops, &s->pci_bus,
   "pci-vpb-config", 0x100);
 sysbus_init_mmio(sbd, &s->mem_config2);
 isa_mmio_setup(&s->isa, 0x010);
 sysbus_init_mmio(sbd, &s->isa);
 
-pci_create_simple(bus, -1, "versatile_pci_host");
+/* TODO Remove once realize propagates to child devices. */
+object_property_set_bool(OBJECT(&s->pci_dev), true, "realized", errp);
 }
 
 static int versatile_pci_host_init(PCIDevice *d)
@@ -134,8 +152,9 @@ static void pci_vpb_class_init(ObjectClass *klass, void 
*data)
 
 static const TypeInfo pci_vpb_info = {
 .name  = TYPE_VERSATILE_PCI,
-.parent= TYPE_SYS_BUS_DEVICE,
+.parent= TYPE_PCI_HOST_BRIDGE,
 .instance_size = sizeof(PCIVPBState),
+.instance_init = pci_vpb_init,
 .class_init= pci_vpb_class_init,
 };
 
-- 
1.7.9.5




[Qemu-devel] [PATCH 10/10] versatile_pci: Expose PCI memory space to system

2013-03-24 Thread Peter Maydell
The VersatilePB's PCI controller exposes the PCI memory space to the
system via three regions controlled by the mapping control registers.
Implement this so that guests can actually use MMIO-BAR PCI cards.

Signed-off-by: Peter Maydell 
---
 hw/arm/realview.c|3 +++
 hw/arm/versatilepb.c |3 +++
 hw/versatile_pci.c   |   72 +-
 3 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/hw/arm/realview.c b/hw/arm/realview.c
index 23c968b..db41525 100644
--- a/hw/arm/realview.c
+++ b/hw/arm/realview.c
@@ -221,6 +221,9 @@ static void realview_init(QEMUMachineInitArgs *args,
 sysbus_mmio_map(busdev, 1, 0x6000); /* PCI self-config */
 sysbus_mmio_map(busdev, 2, 0x6100); /* PCI config */
 sysbus_mmio_map(busdev, 3, 0x6200); /* PCI I/O */
+sysbus_mmio_map(busdev, 4, 0x6300); /* PCI memory window 1 */
+sysbus_mmio_map(busdev, 5, 0x6400); /* PCI memory window 2 */
+sysbus_mmio_map(busdev, 6, 0x6800); /* PCI memory window 3 */
 sysbus_connect_irq(busdev, 0, pic[48]);
 sysbus_connect_irq(busdev, 1, pic[49]);
 sysbus_connect_irq(busdev, 2, pic[50]);
diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
index 9c9bfde..413f0e9 100644
--- a/hw/arm/versatilepb.c
+++ b/hw/arm/versatilepb.c
@@ -228,6 +228,9 @@ static void versatile_init(QEMUMachineInitArgs *args, int 
board_id)
 sysbus_mmio_map(busdev, 1, 0x4100); /* PCI self-config */
 sysbus_mmio_map(busdev, 2, 0x4200); /* PCI config */
 sysbus_mmio_map(busdev, 3, 0x4300); /* PCI I/O */
+sysbus_mmio_map(busdev, 4, 0x4400); /* PCI memory window 1 */
+sysbus_mmio_map(busdev, 5, 0x5000); /* PCI memory window 2 */
+sysbus_mmio_map(busdev, 6, 0x6000); /* PCI memory window 3 */
 sysbus_connect_irq(busdev, 0, sic[27]);
 sysbus_connect_irq(busdev, 1, sic[28]);
 sysbus_connect_irq(busdev, 2, sic[29]);
diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
index a617675..05e1ec8 100644
--- a/hw/versatile_pci.c
+++ b/hw/versatile_pci.c
@@ -20,14 +20,21 @@ typedef struct {
 MemoryRegion controlregs;
 MemoryRegion mem_config;
 MemoryRegion mem_config2;
+/* Containers representing the PCI address spaces */
 MemoryRegion pci_io_space;
+MemoryRegion pci_mem_space;
+/* Alias regions into PCI address spaces which we expose as sysbus regions.
+ * The offsets into pci_mem_space are controlled by the imap registers.
+ */
 MemoryRegion pci_io_window;
+MemoryRegion pci_mem_window[3];
 PCIBus pci_bus;
 PCIDevice pci_dev;
 
 /* Constant for life of device: */
 int realview;
 uint8_t broken_irq_mapping;
+uint32_t mem_win_size[3];
 
 /* Register state: */
 uint32_t imap[3];
@@ -36,10 +43,49 @@ typedef struct {
 uint32_t flags;
 } PCIVPBState;
 
+static void pci_vpb_update_window(PCIVPBState *s, int i)
+{
+/* Adjust the offset of the alias region we use for
+ * the memory window i to account for a change in the
+ * value of the corresponding IMAP register.
+ * Note that the semantics of the IMAP register differ
+ * for realview and versatile variants of the controller.
+ */
+hwaddr offset;
+if (s->realview) {
+/* Top bits of register (masked according to window size) provide
+ * top bits of PCI address.
+ */
+offset = s->imap[i] & ~(s->mem_win_size[i] - 1);
+} else {
+/* Bottom 4 bits of register provide top 4 bits of PCI address */
+offset = s->imap[i] << 28;
+}
+memory_region_set_alias_offset(&s->pci_mem_window[i], offset);
+}
+
+static void pci_vpb_update_all_windows(PCIVPBState *s)
+{
+/* Update all alias windows based on the current register state */
+int i;
+
+for (i = 0; i < 3; i++) {
+pci_vpb_update_window(s, i);
+}
+}
+
+static int pci_vpb_post_load(void *opaque, int version_id)
+{
+PCIVPBState *s = opaque;
+pci_vpb_update_all_windows(s);
+return 0;
+}
+
 static const VMStateDescription pci_vpb_vmstate = {
 .name = "versatile-pci",
 .version_id = 1,
 .minimum_version_id = 1,
+.post_load = pci_vpb_post_load,
 .fields = (VMStateField[]) {
 VMSTATE_UINT32_ARRAY(imap, PCIVPBState, 3),
 VMSTATE_UINT32_ARRAY(smap, PCIVPBState, 3),
@@ -80,6 +126,7 @@ static void pci_vpb_reg_write(void *opaque, hwaddr addr,
 {
 int win = (addr - PCI_IMAP0) >> 2;
 s->imap[win] = val;
+pci_vpb_update_window(s, win);
 break;
 }
 case PCI_SELFID:
@@ -234,6 +281,8 @@ static void pci_vpb_reset(DeviceState *d)
 s->smap[2] = 0;
 s->selfid = 0;
 s->flags = 0;
+
+pci_vpb_update_all_windows(s);
 }
 
 static void pci_vpb_init(Object *obj)
@@ -242,9 +291,10 @@ static void pci_vpb_init(Object *obj)
 PCIVPBState *s = PCI_VPB(obj);
 
 memory_region_init(&s->pci_io_space, "pci_io", 1ULL << 32);
+memory_regi

Re: [Qemu-devel] [PATCH 1/2] char: add qemu_chr_be_is_fe_connected

2013-03-24 Thread Hans de Goede

Hi,

On 03/22/2013 06:11 PM, Anthony Liguori wrote:

Hans de Goede  writes:





If the qemu-char.c code did:

int qemu_chr_fe_write(...) {
 if (!s->fe_open) {
 qemu_chr_fe_open(s);
 }
 ...
}

That would be one thing.  It's a hack, but a more reasonable hack than
doing this in the backend like we do today.


Agreed.


And in fact, herein lies exactly what the problem is.  There is no
"s->fe_open".  And if such a thing did exist, we would note that this is
in fact guest state and that it needs to be taken care of during
migration.


Agreed too, I've prepared a patch set adding fe_open and cleaning up the
whole existing code around the fe_open stuff. I'll send it directly after
this mail.




We could do the same and have a qemu_fe_generic_open for frontends which
does the qemu_chr_fe_open call.


You mean, in qemu_chr_fe_write()?  Yes, I think not doing this bit in
the backend and making it part of qemu-char would be a significant
improvement and would also guide in solving this problem correctly.


I believe that qemu_chr_fe_write is too late, this assumes that the
frontend is always the first one to do a write (assuming fe_open aware
backends won't do anything until the fe_open happens). But what if the
protocol going over the chardev expects the backend to do the first
write? Then the backend will just sit there waiting for the fe_open,
and the frontend will just sit there waiting for the first write before
sending this fe_open.

I believe that your previous comments on qemu_chr_add_handlers being
the closest thing to a fe_open for many frontends is correct, esp. since
some frontends and hw/qdev-properties-system.c do a NULL
qemu_chr_add_handlers when a frontend is being unregistered / removed /
closed. Which gives us a central place to detect frontend closes too.

So this is what I've used in my patch-set.


Also, you can get reset handling for free by unconditionally clearly
s->fe_open with a reset handler.  That would also simplify the
virtio-serial code since it would no longer need the reset logic.


And for me, the most logical thing is to call qemu_chr_fe_open() in
post_load for the device.


With the device I assume you mean the frontend? That is what we originally
suggested and submitted a patch for (for virtio-console) but Amit didn't
like it.


As Avi would say, this is "derived guest state" and derived guest state
has to be recomputed in post_load handlers.


Agreed, which brings us back to the original patch posted a long time
ago which Amit did not like:
http://lists.gnu.org/archive/html/qemu-devel/2012-11/msg02721.html

Amit can you take a second look at this? Note that after the cleanup patchset
which I'll send right after this mail, it will look slightly different, 
something
like:

@@ -653,6 +654,10 @@ static void virtio_serial_post_load_timer_cb(void *opaque)
 send_control_event(port, VIRTIO_CONSOLE_PORT_OPEN,
port->host_connected);
 }
+vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+if (vsc->set_guest_connected) {
+vsc->set_guest_connected(port, port->guest_connected);
+}
 }
 g_free(s->post_load.connected);
 s->post_load.connected = NULL;

Which to me looks like a reasonable thing to do on post-load.

Regards,

Hans



[Qemu-devel] [PATCH 1/8] RFC: chardev frontend open handling cleanup

2013-03-24 Thread Hans de Goede
This patch-series is the result of the
"[PATCH 1/2] char: add qemu_chr_be_is_fe_connected" discussion thread.

This patch series (tries to) make(s) the frontend open concept both more
explicit and generic, and significantly cleans up the surrounding code.

Regards,

Hans



[Qemu-devel] [PATCH 1/8] qemu-char: Rename opened to be_open

2013-03-24 Thread Hans de Goede
Rename the opened variable to be_open to reflect that it contains the
opened state of the backend.

Signed-off-by: Hans de Goede 
---
 hw/usb/dev-serial.c | 2 +-
 hw/usb/redirect.c   | 2 +-
 include/char/char.h | 2 +-
 qemu-char.c | 6 +++---
 spice-qemu-char.c   | 4 ++--
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 47ac8c9..7c314dc 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -495,7 +495,7 @@ static int usb_serial_initfn(USBDevice *dev)
   usb_serial_event, s);
 usb_serial_handle_reset(dev);
 
-if (s->cs->opened && !dev->attached) {
+if (s->cs->be_open && !dev->attached) {
 usb_device_attach(dev);
 }
 return 0;
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index e0df74b..4d565ba 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -272,7 +272,7 @@ static int usbredir_write(void *priv, uint8_t *data, int 
count)
 USBRedirDevice *dev = priv;
 int r;
 
-if (!dev->cs->opened) {
+if (!dev->cs->be_open) {
 return 0;
 }
 
diff --git a/include/char/char.h b/include/char/char.h
index 0326b2a..d801f92 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -74,7 +74,7 @@ struct CharDriverState {
 int idle_tag;
 char *label;
 char *filename;
-int opened;
+int be_open;
 int avail_connections;
 QemuOpts *opts;
 QTAILQ_ENTRY(CharDriverState) next;
diff --git a/qemu-char.c b/qemu-char.c
index 4e011df..32a05af 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -108,10 +108,10 @@ void qemu_chr_be_event(CharDriverState *s, int event)
 /* Keep track if the char device is open */
 switch (event) {
 case CHR_EVENT_OPENED:
-s->opened = 1;
+s->be_open = 1;
 break;
 case CHR_EVENT_CLOSED:
-s->opened = 0;
+s->be_open = 0;
 break;
 }
 
@@ -207,7 +207,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
 
 /* We're connecting to an already opened device, so let's make sure we
also get the open event */
-if (s->opened) {
+if (s->be_open) {
 qemu_chr_generic_open(s);
 }
 }
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 607abb6..613cc64 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -91,8 +91,8 @@ static void vmc_state(SpiceCharDeviceInstance *sin, int 
connected)
 {
 SpiceCharDriver *scd = container_of(sin, SpiceCharDriver, sin);
 
-if ((scd->chr->opened && connected) ||
-(!scd->chr->opened && !connected)) {
+if ((scd->chr->be_open && connected) ||
+(!scd->chr->be_open && !connected)) {
 return;
 }
 
-- 
1.8.1.4




[Qemu-devel] [PATCH 2/8] qemu-char: Rename qemu_chr_generic_open to qemu_chr_be_generic_open

2013-03-24 Thread Hans de Goede
To better reflect that it is for handling a backend being opened.

Signed-off-by: Hans de Goede 
---
 backends/baum.c |  2 +-
 include/char/char.h |  2 +-
 qemu-char.c | 24 
 ui/console.c|  2 +-
 ui/gtk.c|  2 +-
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/backends/baum.c b/backends/baum.c
index d7d658c..ea9ffe8 100644
--- a/backends/baum.c
+++ b/backends/baum.c
@@ -611,7 +611,7 @@ CharDriverState *chr_baum_init(void)
 
 qemu_set_fd_handler(baum->brlapi_fd, baum_chr_read, NULL, baum);
 
-qemu_chr_generic_open(chr);
+qemu_chr_be_generic_open(chr);
 
 return chr;
 
diff --git a/include/char/char.h b/include/char/char.h
index d801f92..dd8f39a 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -235,7 +235,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
IOEventHandler *fd_event,
void *opaque);
 
-void qemu_chr_generic_open(CharDriverState *s);
+void qemu_chr_be_generic_open(CharDriverState *s);
 void qemu_chr_accept_input(CharDriverState *s);
 int qemu_chr_add_client(CharDriverState *s, int fd);
 void qemu_chr_info_print(Monitor *mon, const QObject *ret_data);
diff --git a/qemu-char.c b/qemu-char.c
index 32a05af..55795d7 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -120,7 +120,7 @@ void qemu_chr_be_event(CharDriverState *s, int event)
 s->chr_event(s->handler_opaque, event);
 }
 
-static gboolean qemu_chr_generic_open_bh(gpointer opaque)
+static gboolean qemu_chr_be_generic_open_bh(gpointer opaque)
 {
 CharDriverState *s = opaque;
 qemu_chr_be_event(s, CHR_EVENT_OPENED);
@@ -128,10 +128,10 @@ static gboolean qemu_chr_generic_open_bh(gpointer opaque)
 return FALSE;
 }
 
-void qemu_chr_generic_open(CharDriverState *s)
+void qemu_chr_be_generic_open(CharDriverState *s)
 {
 if (s->idle_tag == 0) {
-s->idle_tag = g_idle_add(qemu_chr_generic_open_bh, s);
+s->idle_tag = g_idle_add(qemu_chr_be_generic_open_bh, s);
 }
 }
 
@@ -208,7 +208,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
 /* We're connecting to an already opened device, so let's make sure we
also get the open event */
 if (s->be_open) {
-qemu_chr_generic_open(s);
+qemu_chr_be_generic_open(s);
 }
 }
 
@@ -482,7 +482,7 @@ static CharDriverState *qemu_chr_open_mux(CharDriverState 
*drv)
 chr->chr_guest_close = NULL;
 
 /* Muxes are always open on creation */
-qemu_chr_generic_open(chr);
+qemu_chr_be_generic_open(chr);
 
 return chr;
 }
@@ -836,7 +836,7 @@ static CharDriverState *qemu_chr_open_fd(int fd_in, int 
fd_out)
 chr->chr_update_read_handler = fd_chr_update_read_handler;
 chr->chr_close = fd_chr_close;
 
-qemu_chr_generic_open(chr);
+qemu_chr_be_generic_open(chr);
 
 return chr;
 }
@@ -1133,7 +1133,7 @@ static void pty_chr_state(CharDriverState *chr, int 
connected)
 pty_chr_rearm_timer(chr, 1000);
 } else {
 if (!s->connected)
-qemu_chr_generic_open(chr);
+qemu_chr_be_generic_open(chr);
 s->connected = 1;
 }
 }
@@ -1549,7 +1549,7 @@ static CharDriverState *qemu_chr_open_pp_fd(int fd)
 chr->chr_close = pp_close;
 chr->opaque = drv;
 
-qemu_chr_generic_open(chr);
+qemu_chr_be_generic_open(chr);
 
 return chr;
 }
@@ -1834,7 +1834,7 @@ static CharDriverState *qemu_chr_open_win_path(const char 
*filename)
 g_free(chr);
 return NULL;
 }
-qemu_chr_generic_open(chr);
+qemu_chr_be_generic_open(chr);
 return chr;
 }
 
@@ -1934,7 +1934,7 @@ static CharDriverState *qemu_chr_open_pipe(ChardevHostdev 
*opts)
 g_free(chr);
 return NULL;
 }
-qemu_chr_generic_open(chr);
+qemu_chr_be_generic_open(chr);
 return chr;
 }
 
@@ -1948,7 +1948,7 @@ static CharDriverState *qemu_chr_open_win_file(HANDLE 
fd_out)
 s->hcom = fd_out;
 chr->opaque = s;
 chr->chr_write = win_chr_write;
-qemu_chr_generic_open(chr);
+qemu_chr_be_generic_open(chr);
 return chr;
 }
 
@@ -2513,7 +2513,7 @@ static void tcp_chr_connect(void *opaque)
 if (s->chan) {
 s->tag = io_add_watch_poll(s->chan, tcp_chr_read_poll, tcp_chr_read, 
chr);
 }
-qemu_chr_generic_open(chr);
+qemu_chr_be_generic_open(chr);
 }
 
 #define IACSET(x,a,b,c) x[0] = a; x[1] = b; x[2] = c;
diff --git a/ui/console.c b/ui/console.c
index eb7a2bc..e84ba8b 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1600,7 +1600,7 @@ static void text_console_do_init(CharDriverState *chr, 
DisplayState *ds)
 s->t_attrib = s->t_attrib_default;
 }
 
-qemu_chr_generic_open(chr);
+qemu_chr_be_generic_open(chr);
 if (chr->init)
 chr->init(chr);
 }
diff --git a/ui/gtk.c b/ui/gtk.c
index 305940d..903b4d4 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1116,7 +1116,7 @@ static GSList *gd_vc_init(GtkDisplayState *s, 
VirtualConsole *vc, int index, GSL
 
 gtk_menu_she

[Qemu-devel] [PATCH 4/8] qemu-char: Automatically do fe_open / fe_close on qemu_chr_add_handlers

2013-03-24 Thread Hans de Goede
Most frontends can't really determine if the guest actually has the frontend
side open. So lets automatically generate fe_open / fe_close as soon as a
frontend becomes ready (as signalled by calling qemu_chr_add_handlers) /
becomes non ready (as signalled by setting all handlers to NULL).

And allow frontends which can actually determine if the guest is listening to
opt-out of this.

Signed-off-by: Hans de Goede 
---
 hw/usb/redirect.c   |  2 --
 hw/virtio-console.c |  1 +
 include/char/char.h |  1 +
 qemu-char.c | 13 +
 4 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 4d565ba..0ddb081 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1310,7 +1310,6 @@ static int usbredir_initfn(USBDevice *udev)
 dev->compatible_speedmask = USB_SPEED_MASK_FULL | USB_SPEED_MASK_HIGH;
 
 /* Let the backend know we are ready */
-qemu_chr_fe_open(dev->cs);
 qemu_chr_add_handlers(dev->cs, usbredir_chardev_can_read,
   usbredir_chardev_read, usbredir_chardev_event, dev);
 
@@ -1334,7 +1333,6 @@ static void usbredir_handle_destroy(USBDevice *udev)
 {
 USBRedirDevice *dev = DO_UPCAST(USBRedirDevice, dev, udev);
 
-qemu_chr_fe_close(dev->cs);
 qemu_chr_delete(dev->cs);
 /* Note must be done after qemu_chr_close, as that causes a close event */
 qemu_bh_delete(dev->chardev_close_bh);
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 8ef76e2..209ea38 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -140,6 +140,7 @@ static int virtconsole_initfn(VirtIOSerialPort *port)
 }
 
 if (vcon->chr) {
+vcon->chr->explicit_fe_open = 1;
 qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
   vcon);
 }
diff --git a/include/char/char.h b/include/char/char.h
index 3174575..27ebbc3 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -76,6 +76,7 @@ struct CharDriverState {
 char *filename;
 int be_open;
 int fe_open;
+int explicit_fe_open;
 int avail_connections;
 QemuOpts *opts;
 QTAILQ_ENTRY(CharDriverState) next;
diff --git a/qemu-char.c b/qemu-char.c
index 554d72f..7c57971 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -194,9 +194,14 @@ void qemu_chr_add_handlers(CharDriverState *s,
IOEventHandler *fd_event,
void *opaque)
 {
+int fe_open;
+
 if (!opaque && !fd_can_read && !fd_read && !fd_event) {
 /* chr driver being released. */
+fe_open = 0;
 ++s->avail_connections;
+} else {
+fe_open = 1;
 }
 s->chr_can_read = fd_can_read;
 s->chr_read = fd_read;
@@ -205,6 +210,14 @@ void qemu_chr_add_handlers(CharDriverState *s,
 if (s->chr_update_read_handler)
 s->chr_update_read_handler(s);
 
+if (!s->explicit_fe_open) {
+if (fe_open) {
+qemu_chr_fe_open(s);
+} else {
+qemu_chr_fe_close(s);
+}
+}
+
 /* We're connecting to an already opened device, so let's make sure we
also get the open event */
 if (s->be_open) {
-- 
1.8.1.4




[Qemu-devel] [PATCH 7/8] virtio-serial: Consolidate guest_open/guest_close into set_guest_connected

2013-03-24 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 hw/virtio-console.c| 23 +--
 hw/virtio-serial-bus.c | 15 +--
 hw/virtio-serial.h |  6 ++
 3 files changed, 12 insertions(+), 32 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index c0d41c5..e28c54f 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -71,26 +71,15 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const 
uint8_t *buf, size_t len)
 return ret;
 }
 
-/* Callback function that's called when the guest opens the port */
-static void guest_open(VirtIOSerialPort *port)
+/* Callback function that's called when the guest opens/closes the port */
+static void set_guest_connected(VirtIOSerialPort *port, int guest_connected)
 {
 VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
 
 if (!vcon->chr) {
 return;
 }
-qemu_chr_fe_set_open(vcon->chr, 1);
-}
-
-/* Callback function that's called when the guest closes the port */
-static void guest_close(VirtIOSerialPort *port)
-{
-VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
-
-if (!vcon->chr) {
-return;
-}
-qemu_chr_fe_set_open(vcon->chr, 0);
+qemu_chr_fe_set_open(vcon->chr, guest_connected);
 }
 
 /* Readiness of the guest to accept data on a port */
@@ -173,8 +162,7 @@ static void virtconsole_class_init(ObjectClass *klass, void 
*data)
 k->init = virtconsole_initfn;
 k->exit = virtconsole_exitfn;
 k->have_data = flush_buf;
-k->guest_open = guest_open;
-k->guest_close = guest_close;
+k->set_guest_connected = set_guest_connected;
 dc->props = virtconsole_properties;
 }
 
@@ -197,8 +185,7 @@ static void virtserialport_class_init(ObjectClass *klass, 
void *data)
 
 k->init = virtconsole_initfn;
 k->have_data = flush_buf;
-k->guest_open = guest_open;
-k->guest_close = guest_close;
+k->set_guest_connected = set_guest_connected;
 dc->props = virtserialport_properties;
 }
 
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index ab7168e..eb7af21 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -372,14 +372,9 @@ static void handle_control_message(VirtIOSerial *vser, 
void *buf, size_t len)
 
 case VIRTIO_CONSOLE_PORT_OPEN:
 port->guest_connected = cpkt.value;
-if (cpkt.value && vsc->guest_open) {
+if (vsc->set_guest_connected) {
 /* Send the guest opened notification if an app is interested */
-vsc->guest_open(port);
-}
-
-if (!cpkt.value && vsc->guest_close) {
-/* Send the guest closed notification if an app is interested */
-vsc->guest_close(port);
+vsc->set_guest_connected(port, cpkt.value);
 }
 break;
 }
@@ -484,9 +479,9 @@ static void guest_reset(VirtIOSerial *vser)
 vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
 if (port->guest_connected) {
 port->guest_connected = false;
-
-if (vsc->guest_close)
-vsc->guest_close(port);
+if (vsc->set_guest_connected) {
+vsc->set_guest_connected(port, false);
+}
 }
 }
 }
diff --git a/hw/virtio-serial.h b/hw/virtio-serial.h
index 484dcfe..516400f 100644
--- a/hw/virtio-serial.h
+++ b/hw/virtio-serial.h
@@ -92,10 +92,8 @@ typedef struct VirtIOSerialPortClass {
 int (*exit)(VirtIOSerialPort *port);
 
 /* Callbacks for guest events */
-/* Guest opened device. */
-void (*guest_open)(VirtIOSerialPort *port);
-/* Guest closed device. */
-void (*guest_close)(VirtIOSerialPort *port);
+/* Guest opened/closed device. */
+void (*set_guest_connected)(VirtIOSerialPort *port, int guest_connected);
 
 /* Guest is now ready to accept data (virtqueues set up). */
 void (*guest_ready)(VirtIOSerialPort *port);
-- 
1.8.1.4




[Qemu-devel] [PATCH 6/8] qemu-char: Consolidate guest_close/guest_open into a set_fe_open callback

2013-03-24 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 include/char/char.h |  3 +--
 qemu-char.c | 10 +++---
 spice-qemu-char.c   | 17 +++--
 3 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/include/char/char.h b/include/char/char.h
index 3c8dd28..1457e80 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -68,8 +68,7 @@ struct CharDriverState {
 void (*chr_close)(struct CharDriverState *chr);
 void (*chr_accept_input)(struct CharDriverState *chr);
 void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
-void (*chr_guest_open)(struct CharDriverState *chr);
-void (*chr_guest_close)(struct CharDriverState *chr);
+void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open);
 void *opaque;
 int idle_tag;
 char *label;
diff --git a/qemu-char.c b/qemu-char.c
index 713c154..5be2ae7 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -487,8 +487,7 @@ static CharDriverState *qemu_chr_open_mux(CharDriverState 
*drv)
 chr->chr_update_read_handler = mux_chr_update_read_handler;
 chr->chr_accept_input = mux_chr_accept_input;
 /* Frontend guest-open / -close notification is not support with muxes */
-chr->chr_guest_open = NULL;
-chr->chr_guest_close = NULL;
+chr->chr_set_fe_open = NULL;
 
 /* Muxes are always open on creation */
 qemu_chr_be_generic_open(chr);
@@ -3395,11 +3394,8 @@ void qemu_chr_fe_set_echo(struct CharDriverState *chr, 
bool echo)
 void qemu_chr_fe_set_open(struct CharDriverState *chr, int fe_open)
 {
 chr->fe_open = fe_open;
-if (fe_open && chr->chr_guest_open) {
-chr->chr_guest_open(chr);
-}
-if (!fe_open && chr->chr_guest_close) {
-chr->chr_guest_close(chr);
+if (chr->chr_set_fe_open) {
+chr->chr_set_fe_open(chr, fe_open);
 }
 }
 
diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 613cc64..ba59374 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -213,16 +213,14 @@ static void spice_chr_close(struct CharDriverState *chr)
 g_free(s);
 }
 
-static void spice_chr_guest_open(struct CharDriverState *chr)
+static void spice_chr_set_fe_open(struct CharDriverState *chr, int fe_open)
 {
 SpiceCharDriver *s = chr->opaque;
-vmc_register_interface(s);
-}
-
-static void spice_chr_guest_close(struct CharDriverState *chr)
-{
-SpiceCharDriver *s = chr->opaque;
-vmc_unregister_interface(s);
+if (fe_open) {
+vmc_register_interface(s);
+} else {
+vmc_unregister_interface(s);
+}
 }
 
 static void print_allowed_subtypes(void)
@@ -256,8 +254,7 @@ static CharDriverState *chr_open(const char *subtype)
 chr->chr_write = spice_chr_write;
 chr->chr_add_watch = spice_chr_add_watch;
 chr->chr_close = spice_chr_close;
-chr->chr_guest_open = spice_chr_guest_open;
-chr->chr_guest_close = spice_chr_guest_close;
+chr->chr_set_fe_open = spice_chr_set_fe_open;
 
 QLIST_INSERT_HEAD(&spice_chars, s, next);
 
-- 
1.8.1.4




[Qemu-devel] [PATCH 3/8] qemu-char: Add fe_open tracking

2013-03-24 Thread Hans de Goede
Add tracking of the fe_open state to struct CharDriverState.

Signed-off-by: Hans de Goede 
---
 include/char/char.h | 1 +
 qemu-char.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/include/char/char.h b/include/char/char.h
index dd8f39a..3174575 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -75,6 +75,7 @@ struct CharDriverState {
 char *label;
 char *filename;
 int be_open;
+int fe_open;
 int avail_connections;
 QemuOpts *opts;
 QTAILQ_ENTRY(CharDriverState) next;
diff --git a/qemu-char.c b/qemu-char.c
index 55795d7..554d72f 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3385,6 +3385,7 @@ void qemu_chr_fe_set_echo(struct CharDriverState *chr, 
bool echo)
 
 void qemu_chr_fe_open(struct CharDriverState *chr)
 {
+chr->fe_open = 1;
 if (chr->chr_guest_open) {
 chr->chr_guest_open(chr);
 }
@@ -3392,6 +3393,7 @@ void qemu_chr_fe_open(struct CharDriverState *chr)
 
 void qemu_chr_fe_close(struct CharDriverState *chr)
 {
+chr->fe_open = 0;
 if (chr->chr_guest_close) {
 chr->chr_guest_close(chr);
 }
-- 
1.8.1.4




[Qemu-devel] [PATCH 5/8] qemu-char: Cleanup: consolidate fe_open/fe_close into fe_set_open

2013-03-24 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 hw/virtio-console.c |  4 ++--
 include/char/char.h | 17 -
 qemu-char.c | 19 +--
 3 files changed, 11 insertions(+), 29 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 209ea38..c0d41c5 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -79,7 +79,7 @@ static void guest_open(VirtIOSerialPort *port)
 if (!vcon->chr) {
 return;
 }
-qemu_chr_fe_open(vcon->chr);
+qemu_chr_fe_set_open(vcon->chr, 1);
 }
 
 /* Callback function that's called when the guest closes the port */
@@ -90,7 +90,7 @@ static void guest_close(VirtIOSerialPort *port)
 if (!vcon->chr) {
 return;
 }
-qemu_chr_fe_close(vcon->chr);
+qemu_chr_fe_set_open(vcon->chr, 0);
 }
 
 /* Readiness of the guest to accept data on a port */
diff --git a/include/char/char.h b/include/char/char.h
index 27ebbc3..3c8dd28 100644
--- a/include/char/char.h
+++ b/include/char/char.h
@@ -129,21 +129,12 @@ void qemu_chr_delete(CharDriverState *chr);
 void qemu_chr_fe_set_echo(struct CharDriverState *chr, bool echo);
 
 /**
- * @qemu_chr_fe_open:
+ * @qemu_chr_fe_set_open:
  *
- * Open a character backend.  This function call is an indication that the
- * front end is ready to begin doing I/O.
+ * Set character frontend open status.  This is an indication that the
+ * front end is ready (or not) to begin doing I/O.
  */
-void qemu_chr_fe_open(struct CharDriverState *chr);
-
-/**
- * @qemu_chr_fe_close:
- *
- * Close a character backend.  This function call indicates that the front end
- * no longer is able to process I/O.  To process I/O again, the front end will
- * call @qemu_chr_fe_open.
- */
-void qemu_chr_fe_close(struct CharDriverState *chr);
+void qemu_chr_fe_set_open(struct CharDriverState *chr, int fe_open);
 
 /**
  * @qemu_chr_fe_printf:
diff --git a/qemu-char.c b/qemu-char.c
index 7c57971..713c154 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -211,11 +211,7 @@ void qemu_chr_add_handlers(CharDriverState *s,
 s->chr_update_read_handler(s);
 
 if (!s->explicit_fe_open) {
-if (fe_open) {
-qemu_chr_fe_open(s);
-} else {
-qemu_chr_fe_close(s);
-}
+qemu_chr_fe_set_open(s, fe_open);
 }
 
 /* We're connecting to an already opened device, so let's make sure we
@@ -3396,18 +3392,13 @@ void qemu_chr_fe_set_echo(struct CharDriverState *chr, 
bool echo)
 }
 }
 
-void qemu_chr_fe_open(struct CharDriverState *chr)
+void qemu_chr_fe_set_open(struct CharDriverState *chr, int fe_open)
 {
-chr->fe_open = 1;
-if (chr->chr_guest_open) {
+chr->fe_open = fe_open;
+if (fe_open && chr->chr_guest_open) {
 chr->chr_guest_open(chr);
 }
-}
-
-void qemu_chr_fe_close(struct CharDriverState *chr)
-{
-chr->fe_open = 0;
-if (chr->chr_guest_close) {
+if (!fe_open && chr->chr_guest_close) {
 chr->chr_guest_close(chr);
 }
 }
-- 
1.8.1.4




[Qemu-devel] [PATCH 8/8] spice-qemu-char: Drop hackish vmc_register on spice_chr_write

2013-03-24 Thread Hans de Goede
Now that the core takes care of fe_open tracking we no longer need this hack.

Signed-off-by: Hans de Goede 
---
 spice-qemu-char.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index ba59374..7e6bd2d 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -184,7 +184,6 @@ static int spice_chr_write(CharDriverState *chr, const 
uint8_t *buf, int len)
 SpiceCharDriver *s = chr->opaque;
 int read_bytes;
 
-vmc_register_interface(s);
 assert(s->datalen == 0);
 s->datapos = buf;
 s->datalen = len;
-- 
1.8.1.4




[Qemu-devel] [PATCH 0/1] usb-serial: Remove double call to qemu_chr_add_handlers( NULL )

2013-03-24 Thread Hans de Goede
usb-serial has a qdev chardev property, and hw/qdev-properties-system.c
already contains:

static void release_chr(Object *obj, const char *name, void *opaque)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
CharDriverState **ptr = qdev_get_prop_ptr(dev, prop);

if (*ptr) {
qemu_chr_add_handlers(*ptr, NULL, NULL, NULL, NULL);
}
}

So doing the qemu_chr_add_handlers(s->cs, NULL, NULL, NULL, NULL); from
the usb handle_destroy function too will lead to it being done twice, which
will result in a wrong value for cs->avail_connections.

Note:
1) I noticed this will working on other stuff, but I've not actually seen this
happening (I did not try to trigger it), so please review carefully.

2) There are other places which are doing a qemu_chr_add_handlers( NULL )
too, but those don't use a qdev chardev property, so this does not apply:

backends/rng-egd.c
hw/serial.c
hw/xen_console.c

backends/rng-egd.c is weird / suspect here since it uses a qdev string
property for the chardev and then uses qemu_chr_find to get it. I wonder
why it is not simply using a chardev property rather then a string property?

Regards,

Hans



[Qemu-devel] [PATCH] usb-serial: Remove double call to qemu_chr_add_handlers( NULL )

2013-03-24 Thread Hans de Goede
usb-serial has a qdev chardev property, and hw/qdev-properties-system.c
already contains:

static void release_chr(Object *obj, const char *name, void *opaque)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
CharDriverState **ptr = qdev_get_prop_ptr(dev, prop);

if (*ptr) {
qemu_chr_add_handlers(*ptr, NULL, NULL, NULL, NULL);
}
}

So doing the qemu_chr_add_handlers(s->cs, NULL, NULL, NULL, NULL); from
the usb handle_destroy function too will lead to it being done twice, which
will result in a wrong value for cs->avail_connections.

Signed-off-by: Hans de Goede 
---
 hw/usb/dev-serial.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c
index 7c314dc..21ddef6 100644
--- a/hw/usb/dev-serial.c
+++ b/hw/usb/dev-serial.c
@@ -410,13 +410,6 @@ static void usb_serial_handle_data(USBDevice *dev, 
USBPacket *p)
 }
 }
 
-static void usb_serial_handle_destroy(USBDevice *dev)
-{
-USBSerialState *s = (USBSerialState *)dev;
-
-qemu_chr_add_handlers(s->cs, NULL, NULL, NULL, NULL);
-}
-
 static int usb_serial_can_read(void *opaque)
 {
 USBSerialState *s = opaque;
@@ -595,7 +588,6 @@ static void usb_serial_class_initfn(ObjectClass *klass, 
void *data)
 uc->handle_reset   = usb_serial_handle_reset;
 uc->handle_control = usb_serial_handle_control;
 uc->handle_data= usb_serial_handle_data;
-uc->handle_destroy = usb_serial_handle_destroy;
 dc->vmsd = &vmstate_usb_serial;
 dc->props = serial_properties;
 }
@@ -623,7 +615,6 @@ static void usb_braille_class_initfn(ObjectClass *klass, 
void *data)
 uc->handle_reset   = usb_serial_handle_reset;
 uc->handle_control = usb_serial_handle_control;
 uc->handle_data= usb_serial_handle_data;
-uc->handle_destroy = usb_serial_handle_destroy;
 dc->vmsd = &vmstate_usb_serial;
 dc->props = braille_properties;
 }
-- 
1.8.1.4




[Qemu-devel] [PATCH 0/1] virtio-serial: propagate guest_connected to the port on post_load

2013-03-24 Thread Hans de Goede
Here is a new attempt on fixing the spicevmc needing the guest_connected
value post-migration, as discussed in various other threads.

Note this patch applies on top of my frontend-open handling cleanup series I
just posted.

Signed-off-by: Hans de Goede 



[Qemu-devel] [PATCH] virtio-serial: propagate guest_connected to the port on post_load

2013-03-24 Thread Hans de Goede
From: Alon Levy 

When migrating a host with with a spice agent running the mouse becomes
non operational after the migration due to the agent state being
inconsistent between the guest and the client.

After migration the spicevmc backend on the destination has never been notified
of the (non 0) guest_connected state. Virtio-serial holds this state
information and migrates it, this patch properly propagates this information
to virtio-console and through that to interested chardev backends.

rhbz #725965

Signed-off-by: Hans de Goede 
---
 hw/virtio-serial-bus.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index eb7af21..a9cb114 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -579,6 +579,7 @@ static void virtio_serial_post_load_timer_cb(void *opaque)
 VirtIOSerial *s = opaque;
 VirtIOSerialPort *port;
 uint8_t host_connected;
+VirtIOSerialPortClass *vsc;
 
 if (!s->post_load) {
 return;
@@ -594,6 +595,10 @@ static void virtio_serial_post_load_timer_cb(void *opaque)
 send_control_event(s, port->id, VIRTIO_CONSOLE_PORT_OPEN,
port->host_connected);
 }
+vsc = VIRTIO_SERIAL_PORT_GET_CLASS(port);
+if (vsc->set_guest_connected) {
+vsc->set_guest_connected(port, port->guest_connected);
+}
 }
 g_free(s->post_load->connected);
 qemu_free_timer(s->post_load->timer);
-- 
1.8.1.4




Re: [Qemu-devel] [PATCH v2] ifname=xxx for -netdev bridge

2013-03-24 Thread Alexandre Kandalintsev
Eric Blake  wrote:
> You also need to add a line documenting this field:
> 
> # @ifname: #optional Set the interface name that will be used (since
> 1.5).

Done. I also added an example with -netdev and -device
(seems this question is not covered in docs).


Signed-off-by: Alexandre Kandalintsev 
---
 include/net/net.h|  1 +
 net/tap.c| 25 -
 qapi-schema.json |  1 +
 qemu-bridge-helper.c | 12 ++--
 qemu-options.hx  | 18 --
 5 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index cb049a1..4b25eb5 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -172,6 +172,7 @@ NetClientState *net_hub_port_find(int hub_id);
 #define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
 #define DEFAULT_BRIDGE_HELPER CONFIG_QEMU_HELPERDIR "/qemu-bridge-helper"
 #define DEFAULT_BRIDGE_INTERFACE "br0"
+#define DEFAULT_BRIDGE_IFNAME "tap%d"
 
 void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
 
diff --git a/net/tap.c b/net/tap.c
index ce79699..d1d5b6a 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -424,11 +424,11 @@ static int recv_fd(int c)
 return len;
 }
 
-static int net_bridge_run_helper(const char *helper, const char *bridge)
+static int net_bridge_run_helper(const char *helper, const char *bridge, const 
char *ifname)
 {
 sigset_t oldmask, mask;
 int pid, status;
-char *args[5];
+char *args[6];
 char **parg;
 int sv[2];
 
@@ -446,6 +446,7 @@ static int net_bridge_run_helper(const char *helper, const 
char *bridge)
 int open_max = sysconf(_SC_OPEN_MAX), i;
 char fd_buf[6+10];
 char br_buf[6+IFNAMSIZ] = {0};
+char ifname_buf[9+IFNAMSIZ] = {0};
 char helper_cmd[PATH_MAX + sizeof(fd_buf) + sizeof(br_buf) + 15];
 
 for (i = 0; i < open_max; i++) {
@@ -459,6 +460,10 @@ static int net_bridge_run_helper(const char *helper, const 
char *bridge)
 
 snprintf(fd_buf, sizeof(fd_buf), "%s%d", "--fd=", sv[1]);
 
+if (ifname) {
+  snprintf(ifname_buf, sizeof(ifname_buf), "%s%s", "--ifname=", 
ifname);
+}
+
 if (strrchr(helper, ' ') || strrchr(helper, '\t')) {
 /* assume helper is a command */
 
@@ -466,8 +471,8 @@ static int net_bridge_run_helper(const char *helper, const 
char *bridge)
 snprintf(br_buf, sizeof(br_buf), "%s%s", "--br=", bridge);
 }
 
-snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s",
- helper, "--use-vnet", fd_buf, br_buf);
+snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s %s",
+ helper, "--use-vnet", fd_buf, br_buf, ifname_buf);
 
 parg = args;
 *parg++ = (char *)"sh";
@@ -486,6 +491,7 @@ static int net_bridge_run_helper(const char *helper, const 
char *bridge)
 *parg++ = (char *)"--use-vnet";
 *parg++ = fd_buf;
 *parg++ = br_buf;
+*parg++ = ifname_buf;
 *parg++ = NULL;
 
 execv(helper, args);
@@ -524,7 +530,7 @@ int net_init_bridge(const NetClientOptions *opts, const 
char *name,
 NetClientState *peer)
 {
 const NetdevBridgeOptions *bridge;
-const char *helper, *br;
+const char *helper, *br, *ifname;
 
 TAPState *s;
 int fd, vnet_hdr;
@@ -534,8 +540,9 @@ int net_init_bridge(const NetClientOptions *opts, const 
char *name,
 
 helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER;
 br = bridge->has_br ? bridge->br : DEFAULT_BRIDGE_INTERFACE;
+ifname = bridge->has_ifname ? bridge->ifname : DEFAULT_BRIDGE_IFNAME;
 
-fd = net_bridge_run_helper(helper, br);
+fd = net_bridge_run_helper(helper, br, ifname);
 if (fd == -1) {
 return -1;
 }
@@ -686,11 +693,12 @@ int net_init_tap(const NetClientOptions *opts, const char 
*name,
 const char *script = NULL; /* suppress wrong "uninit'd use" gcc warning */
 const char *downscript = NULL;
 const char *vhostfdname;
+const char *br;
 char ifname[128];
 
 assert(opts->kind == NET_CLIENT_OPTIONS_KIND_TAP);
 tap = opts->tap;
-queues = tap->has_queues ? tap->queues : 1;
+queues  = tap->has_queues  ? tap->queues  : 1;
 vhostfdname = tap->has_vhostfd ? tap->vhostfd : NULL;
 
 /* QEMU vlans does not support multiqueue tap, in this case peer is set.
@@ -775,8 +783,7 @@ int net_init_tap(const NetClientOptions *opts, const char 
*name,
  "queues=, and fds= are invalid with helper=");
 return -1;
 }
-
-fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE);
+fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE, 
NULL);
 if (fd == -1) {
 return -1;
 }
diff --git a/qapi-schema.json b/qapi-schema.json
index 088f4e1..be2373a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2676,6 +267

Re: [Qemu-devel] qemu-x86_64 on i386 host: SIGSEGV

2013-03-24 Thread Michael Tokarev
24.03.2013 14:59, Peter Maydell wrote:
> On 24 March 2013 10:43, Michael Tokarev  wrote:
>> $ ./x86_64-linux-user/qemu-x86_64 bash64
>> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
>> Segmentation Fault
> 
> Are 64 bit linux-user guests on 32 bit hosts supposed to work?
> I would expect them to be at best pretty unreliable.

What's the reason we build these binaries in that case?

What about qemu-x86_64 on other 32bit arches (arm)?  Is
there a list of combinations (host/target) which are
supposed to work and which don't, somewhere?

>> $ gdb x86_64-linux-user/qemu-x86_64
>> (gdb) ru bash64
>> Program received signal SIGSEGV, Segmentation fault.
>> disas_insn (s=s@entry=0xcf98, pc_start=18446744073699066880)
>> at target-i386/translate.c:4107
>> 4107b = ldub_code(s->pc);
>> (gdb) p *s
>> $1 = {override = -1, prefix = 1484501952, aflag = 1, dflag = 1484503884,
>>   pc = 18446744073699066880, is_jmp = 0, cs_base = 0, pe = 1, code32 = 1,
> 
> PC is FF600400 so either we've messed it up already or this
> is just "64 bit address space doesn't fit in a 32 bit one".

>> Some binaries works - for example, gzip (it prints "qemu: Unsupported 
>> syscall:
>> 202" on the way which is a different issue).
> 
> Yes. That is just the usual "x86 linux-user isn't really supported":
> 202 is TARGET_NR_futex, which works on other target archs but
> won't on x86 until somebody actually fixes support for threaded
> guests in x86 to at least the level it is for other targets.

Maybe we should stop building x86 linux-user completely?

Thanks,

/mjt



[Qemu-devel] [PATCHv2] ibverbs: add a new IBV_ACCESS_GIFT option

2013-03-24 Thread Michael S. Tsirkin
At the moment registering an MR breaks COW.  If the application does not
care that adapter sees stale data (for example, it tracks writes
reregisters and resends), it can use a new IBV_ACCESS_GIFT flag to
prevent registration from breaking COW.

The semantics are similar to that of SPLICE_F_GIFT thus the name.

Signed-off-by: Michael S. Tsirkin 
---

This is compiled but untested. Michael, could you please try
this patch (together with the kernel patch I'm sending separately)
and report whether setting this flag unbreaks overcommit for you?

 include/infiniband/verbs.h | 3 ++-
 man/ibv_reg_mr.3   | 5 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/infiniband/verbs.h b/include/infiniband/verbs.h
index 6acfc81..3290ec9 100644
--- a/include/infiniband/verbs.h
+++ b/include/infiniband/verbs.h
@@ -290,7 +290,8 @@ enum ibv_access_flags {
IBV_ACCESS_REMOTE_WRITE = (1<<1),
IBV_ACCESS_REMOTE_READ  = (1<<2),
IBV_ACCESS_REMOTE_ATOMIC= (1<<3),
-   IBV_ACCESS_MW_BIND  = (1<<4)
+   IBV_ACCESS_MW_BIND  = (1<<4),
+   IBV_ACCESS_GIFT = (1<<6)
 };
 
 struct ibv_pd {
diff --git a/man/ibv_reg_mr.3 b/man/ibv_reg_mr.3
index 7723771..3c302f0 100644
--- a/man/ibv_reg_mr.3
+++ b/man/ibv_reg_mr.3
@@ -34,6 +34,8 @@ describes the desired memory protection attributes; it is 
either 0 or the bitwis
 .B IBV_ACCESS_REMOTE_ATOMIC\fR Enable Remote Atomic Operation Access (if 
supported)
 .TP
 .B IBV_ACCESS_MW_BIND\fR   Enable Memory Window Binding
+.TP
+.B IBV_ACCESS_GIFT\fR  This memory is a gift to the adapter: if memory 
is modified after registration, the local version and data seen by the adapter 
through this region rkey may differ
 .PP
 If
 .B IBV_ACCESS_REMOTE_WRITE
@@ -43,6 +45,9 @@ is set, then
 .B IBV_ACCESS_LOCAL_WRITE
 must be set too.
 .PP
+.B IBV_ACCESS_GIFT
+is only legal with remote or local read access.
+.PP
 Local read access is always enabled for the MR.
 .PP
 .B ibv_dereg_mr()
-- 
MST



[Qemu-devel] [PATCHv2] rdma: add a new IB_ACCESS_GIFT flag

2013-03-24 Thread Michael S. Tsirkin
At the moment registering an MR breaks COW.  This breaks memory
overcommit for users such as KVM: we have a lot of COW pages, e.g.
instances of the zero page or pages shared using KSM.

If the application does not care that adapter sees stale data (for
example, it tracks writes reregisters and resends), it can use a new
IBV_ACCESS_GIFT flag to prevent registration from breaking COW.

The semantics are similar to that of SPLICE_F_GIFT thus the name.

Signed-off-by: Michael S. Tsirkin 
---

Please review and consider for 3.10.

Changes from v1:
rename APP_READONLY to _GIFT: similar to vmsplice's F_GIFT.

 drivers/infiniband/core/umem.c | 21 -
 include/rdma/ib_verbs.h|  9 -
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/core/umem.c b/drivers/infiniband/core/umem.c
index a841123..5dee86d 100644
--- a/drivers/infiniband/core/umem.c
+++ b/drivers/infiniband/core/umem.c
@@ -89,6 +89,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, 
unsigned long addr,
int ret;
int off;
int i;
+   bool gift, writable;
DEFINE_DMA_ATTRS(attrs);
 
if (dmasync)
@@ -96,6 +97,15 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, 
unsigned long addr,
 
if (!can_do_mlock())
return ERR_PTR(-EPERM);
+   /*
+* We ask for writable memory if any access flags other than
+* "remote read" or "gift" are set.  "Local write" and "remote write"
+* obviously require write access.  "Remote atomic" can do
+* things like fetch and add, which will modify memory, and
+* "MW bind" can change permissions by binding a window.
+*/
+   gift  = access & IB_ACCESS_GIFT;
+   writable = access & ~(IB_ACCESS_REMOTE_READ | IB_ACCESS_GIFT);
 
umem = kmalloc(sizeof *umem, GFP_KERNEL);
if (!umem)
@@ -105,14 +115,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, 
unsigned long addr,
umem->length= size;
umem->offset= addr & ~PAGE_MASK;
umem->page_size = PAGE_SIZE;
-   /*
-* We ask for writable memory if any access flags other than
-* "remote read" are set.  "Local write" and "remote write"
-* obviously require write access.  "Remote atomic" can do
-* things like fetch and add, which will modify memory, and
-* "MW bind" can change permissions by binding a window.
-*/
-   umem->writable  = !!(access & ~IB_ACCESS_REMOTE_READ);
+   umem->writable  = writable;
 
/* We assume the memory is from hugetlb until proved otherwise */
umem->hugetlb   = 1;
@@ -152,7 +155,7 @@ struct ib_umem *ib_umem_get(struct ib_ucontext *context, 
unsigned long addr,
ret = get_user_pages(current, current->mm, cur_base,
 min_t(unsigned long, npages,
   PAGE_SIZE / sizeof (struct page *)),
-1, !umem->writable, page_list, vma_list);
+!gift, !umem->writable, page_list, 
vma_list);
 
if (ret < 0)
goto out;
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 98cc4b2..2e6e13c 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -871,7 +871,14 @@ enum ib_access_flags {
IB_ACCESS_REMOTE_READ   = (1<<2),
IB_ACCESS_REMOTE_ATOMIC = (1<<3),
IB_ACCESS_MW_BIND   = (1<<4),
-   IB_ZERO_BASED   = (1<<5)
+   IB_ZERO_BASED   = (1<<5),
+   /*
+* IB_ACCESS_GIFT: This memory is a gift to the adapter.  If memory is
+* modified after registration, the local version and data seen by the
+* adapter through this region rkey may differ.
+* Only legal with IB_ACCESS_REMOTE_READ or no permissions.
+*/
+   IB_ACCESS_GIFT  = (1<<6)
 };
 
 struct ib_phys_buf {
-- 
MST



Re: [Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Aurelien Jarno
On Sun, Mar 24, 2013 at 11:32:30AM +, Peter Maydell wrote:
> This patch series fixes a number of serious bugs in our emulation of
> the PCI controller found on VersatilePB and the early Realview boards:
>  * our interrupt mapping was totally wrong
>  * we weren't implementing the PCI memory windows
>  * the I/O window wasn't mapped on VersatilePB
>  * realview mapped things at the wrong addresses
>  * we didn't implement the controller's registers at all
> It also updates to some reasonable approximation to QOM best practice,
> including subclassing pci_host rather than doing everything by hand.
> 
> I haven't implemented support for the SMAP registers (which control
> how the controller converts accesses made by bus-mastering PCI
> devices into system addresses). For the moment we rely on the fact
> that Linux always maps things 1:1. (It wouldn't be too hard to add
> SMAP support but it requires changing QEMU's pci code to allow a
> controller to pass in a MemoryRegion* for DMA to use instead of
> the system address space, so I prefer to leave that for another series.)
> 
> Patchset tested on both versatilepb and realview, using a set of
> Linux kernel patches written by Arnd Bergmann:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-October/029040.html
> which were in turn tested against real PB926 and PB1176 hardware.
> 
> 
>  * WARNING WARNING *
> 
> This patchset will break any use of PCI (including the default SCSI
> card) on versatilepb with current Linux kernels, because those kernels

Do you mean Versatile/PB and not Versatile/AB, or actually both?

> have the matching bug in interrupt mapping to old QEMU.

How is real hardware working with this bug?

> I've provided a property for enabling the old broken IRQ mapping;
> this can be enabled with the command line option:
>   -global versatile_pci.broken-irq-mapping=1
> 
> (If anybody wants to suggest a better way of handling this please do.)

Do you have a pointer to the corresponding kernel patch? Is it possible
to get the kernel to detect if it should use the correct or the broken 
IRQ mapping?


> Peter Maydell (10):
>   versatile_pci: Fix hardcoded tabs
>   versatile_pci: Expose PCI I/O region on Versatile PB
>   versatile_pci: Update to realize and instance init functions
>   versatile_pci: Change to subclassing TYPE_PCI_HOST_BRIDGE
>   versatile_pci: Use separate PCI I/O space rather than system I/O space
>   versatile_pci: Put the host bridge PCI device at slot 29
>   versatile_pci: Implement the correct PCI IRQ mapping
>   versatile_pci: Implement the PCI controller's control registers
>   arm/realview: Fix mapping of PCI regions
>   versatile_pci: Expose PCI memory space to system
> 
>  hw/arm/realview.c|   22 +--
>  hw/arm/versatilepb.c |   11 +-
>  hw/versatile_pci.c   |  368 
> --
>  3 files changed, 344 insertions(+), 57 deletions(-)
> 
> -- 
> 1.7.9.5
> 
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Peter Maydell
On 24 March 2013 15:58, Aurelien Jarno  wrote:
> On Sun, Mar 24, 2013 at 11:32:30AM +, Peter Maydell wrote:
>> This patch series fixes a number of serious bugs in our emulation of
>> the PCI controller found on VersatilePB and the early Realview boards:

>> Patchset tested on both versatilepb and realview, using a set of
>> Linux kernel patches written by Arnd Bergmann:
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-October/029040.html
>> which were in turn tested against real PB926 and PB1176 hardware.
>>
>>
>>  * WARNING WARNING *
>>
>> This patchset will break any use of PCI (including the default SCSI
>> card) on versatilepb with current Linux kernels, because those kernels
>
> Do you mean Versatile/PB and not Versatile/AB, or actually both?

I mean PB. The AB doesn't have a PCI controller (though we incorrectly
model it as having one; I suppose we could patch QEMU to stop doing
that).

>> have the matching bug in interrupt mapping to old QEMU.
>
> How is real hardware working with this bug?

It doesn't. Unless you apply Arnd's patches, PCI support
on real hardware is flat out broken. (This was never noticed
because (a) real hardware is getting rare by now and (b) the
PCI backplane is even rarer.)

>> I've provided a property for enabling the old broken IRQ mapping;
>> this can be enabled with the command line option:
>>   -global versatile_pci.broken-irq-mapping=1
>>
>> (If anybody wants to suggest a better way of handling this please do.)
>
> Do you have a pointer to the corresponding kernel patch? Is it possible
> to get the kernel to detect if it should use the correct or the broken
> IRQ mapping?

I linked to the kernel patches above. It might I guess be possible
to identify a fixed QEMU (if nothing else we could put something
into QEMU that was identifiable); that still leaves existing kernels
breaking, though.

-- PMM



[Qemu-devel] [PATCHv2 0/6] ARM dump-guest-memory support

2013-03-24 Thread Rabin Vincent
A second patchset to add dump-guest-memory support for ARM.

This version of the patchset addresses the following comments from the
previous posting, and also adds some more patches to the core dump code
(patch #4 and #6):

 - memset prstatus to 0 in x86_64_write_elf64_note()
 - handle big endian in dump_write_elf_note()
 - Save CPSR in ARM prstatus
 - set correct ELF endianness for ARM BE 

Rabin Vincent (6):
  dump: create writable files
  dump: extract out note helper
  dump: extract out get note size function
  dump: fix up memory mapping dependencies / stub
  target-arm: add dump-guest-memory support
  dump: fix memory region handling

 Makefile.target  |3 +-
 configure|2 +-
 dump.c   |   88 --
 include/exec/memory.h|7 ++
 include/sysemu/dump.h|6 ++
 memory.c |   12 +++
 memory_mapping-stub.c|5 --
 memory_mapping.c |6 +-
 target-arm/Makefile.objs |2 +-
 target-arm/arch_dump.c   |   61 +
 target-i386/arch_dump.c  |  222 +++---
 11 files changed, 223 insertions(+), 191 deletions(-)
 create mode 100644 target-arm/arch_dump.c

-- 
1.7.10.4




[Qemu-devel] [PATCHv2 1/6] dump: create writable files

2013-03-24 Thread Rabin Vincent
The files dump-guest-memory are created as read-only even for the owner.
This non-standard behaviour makes it annoying to deal with the dump
files (eg. rm -f is needed to delete them or saving a new dump by
overwriting the previous one is not possible).  Change the code to
generate files with write permissions set.  If someone requires
read-only files to be created, they can achieve it by setting umask.

Signed-off-by: Rabin Vincent 
---
 dump.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dump.c b/dump.c
index a25f509..8dd86b4 100644
--- a/dump.c
+++ b/dump.c
@@ -841,7 +841,8 @@ void qmp_dump_guest_memory(bool paging, const char *file, 
bool has_begin,
 #endif
 
 if  (strstart(file, "file:", &p)) {
-fd = qemu_open(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, S_IRUSR);
+fd = qemu_open(p, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY,
+   S_IRUSR | S_IWUSR);
 if (fd < 0) {
 error_set(errp, QERR_OPEN_FILE_FAILED, p);
 return;
-- 
1.7.10.4




[Qemu-devel] [PATCHv2 3/6] dump: extract out get note size function

2013-03-24 Thread Rabin Vincent
Extract out the ELF note size function from i386 so we can use it from
other targets.

Signed-off-by: Rabin Vincent 
---
 dump.c  |   15 +++
 include/sysemu/dump.h   |2 ++
 target-i386/arch_dump.c |   14 ++
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/dump.c b/dump.c
index c5e009a..4b7d76c 100644
--- a/dump.c
+++ b/dump.c
@@ -465,6 +465,21 @@ static hwaddr get_offset(hwaddr phys_addr,
 return -1;
 }
 
+size_t dump_get_note_size(int class, const char *name, size_t descsz)
+{
+int name_size = strlen(name) + 1;
+int note_head_size;
+
+if (class == ELFCLASS32) {
+note_head_size = sizeof(Elf32_Nhdr);
+} else {
+note_head_size = sizeof(Elf64_Nhdr);
+}
+
+return ((note_head_size + 3) / 4 + (name_size + 3) / 4
++ (descsz + 3) / 4) * 4;
+}
+
 int dump_write_elf_note(int class, const char *name, uint32_t type,
 void *desc, size_t descsz,
 write_core_dump_function f, void *opaque)
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index b07816a..a06b149 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -36,4 +36,6 @@ int dump_write_elf_note(int class, const char *name, uint32_t 
type, void *desc,
 size_t descsz, write_core_dump_function f,
 void *opaque);
 
+size_t dump_get_note_size(int class, const char *name, size_t descsz);
+
 #endif
diff --git a/target-i386/arch_dump.c b/target-i386/arch_dump.c
index eea7f7f..49fa024 100644
--- a/target-i386/arch_dump.c
+++ b/target-i386/arch_dump.c
@@ -307,18 +307,10 @@ int cpu_get_dump_info(ArchDumpInfo *info)
 
 ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
 {
-int name_size = 5; /* "CORE" or "QEMU" */
 size_t elf_note_size = 0;
 size_t qemu_note_size = 0;
 int elf_desc_size = 0;
 int qemu_desc_size = 0;
-int note_head_size;
-
-if (class == ELFCLASS32) {
-note_head_size = sizeof(Elf32_Nhdr);
-} else {
-note_head_size = sizeof(Elf64_Nhdr);
-}
 
 if (machine == EM_386) {
 elf_desc_size = sizeof(x86_elf_prstatus);
@@ -330,10 +322,8 @@ ssize_t cpu_get_note_size(int class, int machine, int 
nr_cpus)
 #endif
 qemu_desc_size = sizeof(QEMUCPUState);
 
-elf_note_size = ((note_head_size + 3) / 4 + (name_size + 3) / 4 +
- (elf_desc_size + 3) / 4) * 4;
-qemu_note_size = ((note_head_size + 3) / 4 + (name_size + 3) / 4 +
-  (qemu_desc_size + 3) / 4) * 4;
+elf_note_size = dump_get_note_size(class, "CORE", elf_desc_size);
+qemu_note_size = dump_get_note_size(class, "QEMU", qemu_desc_size);
 
 return (elf_note_size + qemu_note_size) * nr_cpus;
 }
-- 
1.7.10.4




[Qemu-devel] [PATCHv2 2/6] dump: extract out note helper

2013-03-24 Thread Rabin Vincent
Make a common helper function out of the x86 code to add ELF notes.

Signed-off-by: Rabin Vincent 
---
 dump.c  |   51 
 include/sysemu/dump.h   |4 +
 target-i386/arch_dump.c |  208 +++
 3 files changed, 104 insertions(+), 159 deletions(-)

diff --git a/dump.c b/dump.c
index 8dd86b4..c5e009a 100644
--- a/dump.c
+++ b/dump.c
@@ -465,6 +465,57 @@ static hwaddr get_offset(hwaddr phys_addr,
 return -1;
 }
 
+int dump_write_elf_note(int class, const char *name, uint32_t type,
+void *desc, size_t descsz,
+write_core_dump_function f, void *opaque)
+{
+DumpState *s = opaque;
+int endian = s->dump_info.d_endian;
+Elf64_Nhdr *note64;
+Elf32_Nhdr *note32;
+void *note;
+char *buf;
+size_t note_size, name_size, note_head_size;
+int ret;
+
+name_size = strlen(name) + 1;
+
+if (class == ELFCLASS32) {
+note_head_size = sizeof(Elf32_Nhdr);
+} else {
+note_head_size = sizeof(Elf64_Nhdr);
+}
+note_size = ((note_head_size + 3) / 4 + (name_size + 3) / 4 +
+(descsz + 3) / 4) * 4;
+note = g_malloc(note_size);
+
+memset(note, 0, note_size);
+if (class == ELFCLASS32) {
+note32 = note;
+note32->n_namesz = cpu_convert_to_target32(name_size, endian);
+note32->n_descsz = cpu_convert_to_target32(descsz, endian);
+note32->n_type = cpu_convert_to_target32(type, endian);
+} else {
+note64 = note;
+note64->n_namesz = cpu_convert_to_target64(name_size, endian);
+note64->n_descsz = cpu_convert_to_target64(descsz, endian);
+note64->n_type = cpu_convert_to_target64(type, endian);
+}
+buf = note;
+buf += ((note_head_size + 3) / 4) * 4;
+memcpy(buf, name, name_size);
+buf += ((name_size + 3) / 4) * 4;
+memcpy(buf, desc, descsz);
+
+ret = f(note, note_size, opaque);
+g_free(note);
+if (ret < 0) {
+return -1;
+}
+
+return 0;
+}
+
 static int write_elf_loads(DumpState *s)
 {
 hwaddr offset;
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index e25b7cf..b07816a 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -32,4 +32,8 @@ int cpu_write_elf32_qemunote(write_core_dump_function f, 
CPUArchState *env,
 int cpu_get_dump_info(ArchDumpInfo *info);
 ssize_t cpu_get_note_size(int class, int machine, int nr_cpus);
 
+int dump_write_elf_note(int class, const char *name, uint32_t type, void *desc,
+size_t descsz, write_core_dump_function f,
+void *opaque);
+
 #endif
diff --git a/target-i386/arch_dump.c b/target-i386/arch_dump.c
index 2cd2f7f..eea7f7f 100644
--- a/target-i386/arch_dump.c
+++ b/target-i386/arch_dump.c
@@ -38,66 +38,43 @@ static int x86_64_write_elf64_note(write_core_dump_function 
f,
CPUArchState *env, int id,
void *opaque)
 {
-x86_64_user_regs_struct regs;
-Elf64_Nhdr *note;
-char *buf;
-int descsz, note_size, name_size = 5;
-const char *name = "CORE";
-int ret;
-
-regs.r15 = env->regs[15];
-regs.r14 = env->regs[14];
-regs.r13 = env->regs[13];
-regs.r12 = env->regs[12];
-regs.r11 = env->regs[11];
-regs.r10 = env->regs[10];
-regs.r9  = env->regs[9];
-regs.r8  = env->regs[8];
-regs.rbp = env->regs[R_EBP];
-regs.rsp = env->regs[R_ESP];
-regs.rdi = env->regs[R_EDI];
-regs.rsi = env->regs[R_ESI];
-regs.rdx = env->regs[R_EDX];
-regs.rcx = env->regs[R_ECX];
-regs.rbx = env->regs[R_EBX];
-regs.rax = env->regs[R_EAX];
-regs.rip = env->eip;
-regs.eflags = env->eflags;
-
-regs.orig_rax = 0; /* FIXME */
-regs.cs = env->segs[R_CS].selector;
-regs.ss = env->segs[R_SS].selector;
-regs.fs_base = env->segs[R_FS].base;
-regs.gs_base = env->segs[R_GS].base;
-regs.ds = env->segs[R_DS].selector;
-regs.es = env->segs[R_ES].selector;
-regs.fs = env->segs[R_FS].selector;
-regs.gs = env->segs[R_GS].selector;
-
-descsz = sizeof(x86_64_elf_prstatus);
-note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
-(descsz + 3) / 4) * 4;
-note = g_malloc(note_size);
-
-memset(note, 0, note_size);
-note->n_namesz = cpu_to_le32(name_size);
-note->n_descsz = cpu_to_le32(descsz);
-note->n_type = cpu_to_le32(NT_PRSTATUS);
-buf = (char *)note;
-buf += ((sizeof(Elf64_Nhdr) + 3) / 4) * 4;
-memcpy(buf, name, name_size);
-buf += ((name_size + 3) / 4) * 4;
-memcpy(buf + 32, &id, 4); /* pr_pid */
-buf += descsz - sizeof(x86_64_user_regs_struct)-sizeof(target_ulong);
-memcpy(buf, ®s, sizeof(x86_64_user_regs_struct));
-
-ret = f(note, note_size, opaque);
-g_free(note);
-if (ret < 0) {
-return -1;
-}
-
-return 0;
+x86_64_elf_prstatus prstatus;
+
+memset(&prstat

[Qemu-devel] [PATCHv2 4/6] dump: fix up memory mapping dependencies / stub

2013-03-24 Thread Rabin Vincent
dump.c won't build without the functions from memory_mapping.c (and
memory_mapping-stub.c does not help there), so build memory_mapping.c
when CONFIG_HAVE_CORE_DUMP is set.

  dump.c:84: undefined reference to `memory_mapping_list_free'
  dump.c:819: undefined reference to `memory_mapping_list_init'
  dump.c:827: undefined reference to `memory_mapping_filter'

Allow memory_mapping-stub.c to instead be used for targets which do not
set CONFIG_HAVE_GET_MEMORY_MAPPING.

Signed-off-by: Rabin Vincent 
---
 Makefile.target   |3 +--
 memory_mapping-stub.c |5 -
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 2bd6d14..629f48a 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -115,8 +115,7 @@ obj-$(CONFIG_FDT) += device_tree.o
 obj-$(CONFIG_KVM) += kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-y += memory.o savevm.o cputlb.o
-obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
-obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
+obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o memory_mapping.o
 obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
 obj-$(CONFIG_NO_CORE_DUMP) += dump-stub.o
 LIBS+=-lz
diff --git a/memory_mapping-stub.c b/memory_mapping-stub.c
index 24d5d67..6dd9e36 100644
--- a/memory_mapping-stub.c
+++ b/memory_mapping-stub.c
@@ -15,11 +15,6 @@
 #include "exec/cpu-all.h"
 #include "sysemu/memory_mapping.h"
 
-int qemu_get_guest_memory_mapping(MemoryMappingList *list)
-{
-return -2;
-}
-
 int cpu_get_memory_mapping(MemoryMappingList *list,

  CPUArchState *env)
 {
-- 
1.7.10.4




[Qemu-devel] [PATCHv2 6/6] dump: fix memory region handling

2013-03-24 Thread Rabin Vincent
RAMBlock.offset does not provide the physical address of the memory
region.  This is available in the MemoryRegion's address.  The wrong
usage leads to incorrect physical addreses in the ELF.  Fix it.

Signed-off-by: Rabin Vincent 
---
 dump.c|   19 +++
 include/exec/memory.h |7 +++
 memory.c  |   12 
 memory_mapping.c  |6 --
 4 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/dump.c b/dump.c
index 4b7d76c..4b0353a 100644
--- a/dump.c
+++ b/dump.c
@@ -16,6 +16,7 @@
 #include "cpu.h"
 #include "exec/cpu-all.h"
 #include "exec/hwaddr.h"
+#include "exec/memory.h"
 #include "monitor/monitor.h"
 #include "sysemu/kvm.h"
 #include "sysemu/dump.h"
@@ -432,26 +433,28 @@ static hwaddr get_offset(hwaddr phys_addr,
 }
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+hwaddr baddr = memory_region_get_addr(block->mr);
+
 if (s->has_filter) {
-if (block->offset >= s->begin + s->length ||
-block->offset + block->length <= s->begin) {
+if (baddr >= s->begin + s->length ||
+baddr + block->length <= s->begin) {
 /* This block is out of the range */
 continue;
 }
 
-if (s->begin <= block->offset) {
-start = block->offset;
+if (s->begin <= baddr) {
+start = baddr;
 } else {
 start = s->begin;
 }
 
-size_in_block = block->length - (start - block->offset);
-if (s->begin + s->length < block->offset + block->length) {
-size_in_block -= block->offset + block->length -
+size_in_block = block->length - (start - baddr);
+if (s->begin + s->length < baddr + block->length) {
+size_in_block -= baddr + block->length -
  (s->begin + s->length);
 }
 } else {
-start = block->offset;
+start = baddr;
 size_in_block = block->length;
 }
 
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2322732..9227190 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -665,6 +665,13 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr,
  unsigned priority);
 
 /**
+ * memory_region_get_addr: Get the address of a memory region
+ *
+ * @mr: the memory region
+ */
+hwaddr memory_region_get_addr(MemoryRegion *mr);
+
+/**
  * memory_region_get_ram_addr: Get the ram address associated with a memory
  * region
  *
diff --git a/memory.c b/memory.c
index 92a2196..f90fd19 100644
--- a/memory.c
+++ b/memory.c
@@ -1427,6 +1427,18 @@ void memory_region_set_alias_offset(MemoryRegion *mr, 
hwaddr offset)
 memory_region_transaction_commit();
 }
 
+hwaddr memory_region_get_addr(MemoryRegion *mr)
+{
+hwaddr addr = 0;
+
+while (mr) {
+   addr += mr->addr;
+   mr = mr->parent;
+}
+
+return addr;
+}
+
 ram_addr_t memory_region_get_ram_addr(MemoryRegion *mr)
 {
 return mr->ram_addr;
diff --git a/memory_mapping.c b/memory_mapping.c
index ff45b3a..cf0751c 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -13,6 +13,7 @@
 
 #include "cpu.h"
 #include "exec/cpu-all.h"
+#include "exec/memory.h"
 #include "sysemu/memory_mapping.h"
 
 static void memory_mapping_list_add_mapping_sorted(MemoryMappingList *list,
@@ -201,7 +202,7 @@ int qemu_get_guest_memory_mapping(MemoryMappingList *list)
  * address.
  */
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-offset = block->offset;
+offset = memory_region_get_addr(block->mr);
 length = block->length;
 create_new_memory_mapping(list, offset, offset, length);
 }
@@ -214,7 +215,8 @@ void qemu_get_guest_simple_memory_mapping(MemoryMappingList 
*list)
 RAMBlock *block;
 
 QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-create_new_memory_mapping(list, block->offset, 0, block->length);
+create_new_memory_mapping(list, memory_region_get_addr(block->mr),
+  0, block->length);
 }
 }
 
-- 
1.7.10.4




[Qemu-devel] [PATCHv2 5/6] target-arm: add dump-guest-memory support

2013-03-24 Thread Rabin Vincent
Enable support for the dump-guest-memory monitor command for ARM.

Cc: Peter Maydell 
Signed-off-by: Rabin Vincent 
---
 configure|2 +-
 target-arm/Makefile.objs |2 +-
 target-arm/arch_dump.c   |   61 ++
 3 files changed, 63 insertions(+), 2 deletions(-)
 create mode 100644 target-arm/arch_dump.c

diff --git a/configure b/configure
index 46a7594..f35786d 100755
--- a/configure
+++ b/configure
@@ -4184,7 +4184,7 @@ if test "$target_softmmu" = "yes" ; then
   echo "CONFIG_SOFTMMU=y" >> $config_target_mak
   echo "LIBS+=$libs_softmmu $target_libs_softmmu" >> $config_target_mak
   case "$target_arch2" in
-i386|x86_64)
+arm|i386|x86_64)
   echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
   esac
 fi
diff --git a/target-arm/Makefile.objs b/target-arm/Makefile.objs
index d89b57c..93baa12 100644
--- a/target-arm/Makefile.objs
+++ b/target-arm/Makefile.objs
@@ -1,5 +1,5 @@
 obj-y += arm-semi.o
-obj-$(CONFIG_SOFTMMU) += machine.o
+obj-$(CONFIG_SOFTMMU) += machine.o arch_dump.o
 obj-$(CONFIG_KVM) += kvm.o
 obj-y += translate.o op_helper.o helper.o cpu.o
 obj-y += neon_helper.o iwmmxt_helper.o
diff --git a/target-arm/arch_dump.c b/target-arm/arch_dump.c
new file mode 100644
index 000..e568ffb
--- /dev/null
+++ b/target-arm/arch_dump.c
@@ -0,0 +1,61 @@
+#include "cpu.h"
+#include "sysemu/dump.h"
+#include "elf.h"
+
+typedef struct {
+char pad1[24];
+uint32_t pid;
+char pad2[44];
+uint32_t regs[18];
+char pad3[4];
+} arm_elf_prstatus;
+
+int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
+ int cpuid, void *opaque)
+{
+return -1;
+}
+
+int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
+ int cpuid, void *opaque)
+{
+arm_elf_prstatus prstatus = {.pid = cpuid};
+
+memcpy(&(prstatus.regs), env->regs, sizeof(env->regs));
+prstatus.regs[16] = cpsr_read(env);
+
+return dump_write_elf_note(ELFCLASS32, "CORE", NT_PRSTATUS,
+   &prstatus, sizeof(prstatus),
+   f, opaque);
+}
+
+int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env,
+ void *opaque)
+{
+return -1;
+}
+
+int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
+ void *opaque)
+{
+return 0;
+}
+
+int cpu_get_dump_info(ArchDumpInfo *info)
+{
+info->d_machine = EM_ARM;
+#ifdef TARGET_WORDS_BIGENDIAN
+info->d_endian = ELFDATA2MSB;
+#else
+info->d_endian = ELFDATA2LSB;
+#endif
+info->d_class = ELFCLASS32;
+
+return 0;
+}
+
+ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
+{
+return nr_cpus * dump_get_note_size(ELFCLASS32, "CORE",
+sizeof(arm_elf_prstatus));
+}
-- 
1.7.10.4




Re: [Qemu-devel] [PATCH v2] target-i386: Improve x86_cpu_list output

2013-03-24 Thread Andreas Färber
Am 27.02.2013 10:37, schrieb Jan Kiszka:
> On 2013-02-27 10:33, Andreas Färber wrote:
>> Am 27.02.2013 10:15, schrieb Jan Kiszka:
>>> Several issues fixed:
>>>  - We were missing a bunch of feature lists. Fix this by simply dumping
>>>the meta list feature_word_info.
>>>  - kvm_enabled() cannot be true at this point because accelerators are
>>>initialized much later during init. Also, hiding this makes it very
>>>hard to discover for users. Simply dump unconditionally if CONFIG_KVM
>>>is set.
>>>  - Add explanation for "host" CPU type.
>>>
>>> Signed-off-by: Jan Kiszka 
>>> ---
>>>
>>> Changes in v2:
>>>  - Do not dump "host" type if CONFIG_KVM is not set
>>>  - Explain that "host" depends on KVM mode
>>
>> I had requested on v1 to not fix multiple issues in one patch, but I can
>> split it myself on Friday if there's no other issues.
> 
> Sorry, missed that. But I also see no point in splitting up in this
> case, specifically as we no agree on the result.

Thanks, applied to qom-cpu as two patches (clean git-checkout -p split):
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Sorry for taking so long.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PULL 0/5] ipxe: add efi support

2013-03-24 Thread Jan Kiszka
On 2013-03-18 12:12, Gerd Hoffmann wrote:
>   Hi,
> 
> This pull adds efi support to our nic roms.
> 
> Patches have been posted here for review:
> https://lists.nongnu.org/archive/html/qemu-devel/2013-03/msg00344.html
> 
> As there have been no changes (other than a rebase to master)
> I'm not posting the patches again.
> 
> please pull,
>   Gerd
> 
> The following changes since commit 225dc991b03f0f034aa348f5cf499de9d0979107:
> 
>   s390: Fix cpu refactoring fallout. (2013-03-17 20:01:31 +)
> 
> are available in the git repository at:
> 
>   git://git.kraxel.org/qemu ipxe.2
> 
> for you to fetch changes up to c45e5b5b30ac1f5505725a7b36e68cedfce4f01f:
> 
>   Switch to efi-enabled nic roms by default (2013-03-18 10:21:56 +0100)
> 
> 
> Gerd Hoffmann (5):
>   Add Makefile rules to build nic rom binaries
>   Update ipxe submodule to latest master
>   Add Makefile rules to build nic rom binaries with efi support
>   Add efi rom binaries
>   Switch to efi-enabled nic roms by default

At least the e1000 image has that annoying configuration prompt enabled.
Please fix, it prolongs the boot for everyone by several seconds!

Thanks,
Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 00/19] chardev flow control

2013-03-24 Thread Jan Kiszka
On 2013-02-18 22:47, Anthony Liguori wrote:
> This series implements an idea from Paolo to introduce flow control
> in the char layer by converting all char backends to use GIOChannels
> internally.  Then we can just use the existing IO watch support in glib
> to implement flow control.
> 
> This is based on a rebased version of an old series of mine plus a
> number of improvements from Amit Shah.

There is a problem with these changes, maybe just a small bug but I do
not see a simple solution yet:

If an input channel signals readiness but the frontend has no space
(can_read returns 0), the main loop will start busy-waiting, causing
100% CPU load.

It's trivial to reproduce: qemu-system-x86_64 -serial stdio -S, and then
hit a key twice on that console.

Jan




signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v2] gtk: Fix accelerator filtering

2013-03-24 Thread Jan Kiszka
On 2013-02-25 16:44, Jan Kiszka wrote:
> On 2013-02-25 16:39, Anthony Liguori wrote:
>> Jan Kiszka  writes:
>>
>>> This is in fact very simply: When the input in grabbed, everything
>>> should be exclusively passed to the guest - except it has our magic
>>> CTRL-ALT modifier set. Then let GTK filter out those accels that are in
>>> use. When checking the modifier state, we just need to filter out NUM
>>> and CAPS lock.
>>
>> Can you explain what you're fixing?
> 
> That it's not filtering what it is supposed to.
> 
>>
>> We shouldn't hard code modifiers like this.  The reason you give
>> accelerators paths like this is so that they can be overridden by a
>> user.
>>
>> That's why I filtered by path.  Once we're running, we shouldn't assume
>> that accelerators use the modifiers we started with.
> 
> Your path-based filtering does not work as it uses an unsupported
> internal function (see my other mail).
> 
> We can make the modifier configurable via QEMU means (command line
> parameter, gconfig, whatever). But let's get the basics working first.

The bug still exists, my patch still applies. Unless you have some idea
for a better solution, please apply this for now so that CTRL-q inside a
guest doesn't kill more kittens.

Jan




signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v3] gtk: Release modifier when graphic console loses keyboard focus

2013-03-24 Thread Jan Kiszka
From: Jan Kiszka 

This solves, e.g., sticky ALT when selecting a GTK menu, switching to a
different window or selecting a different virtual console.

Signed-off-by: Jan Kiszka 
---

Changes in v3:
 - rebased

 ui/gtk.c |   53 +++--
 1 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index a249059..c04656a 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -102,6 +102,11 @@ static inline void gdk_drawable_get_size(GdkWindow *w, 
gint *ww, gint *wh)
 #define IGNORE_MODIFIER_MASK \
 (GDK_MODIFIER_MASK & ~(GDK_LOCK_MASK | GDK_MOD2_MASK))
 
+static const int modifier_keycode[] = {
+/* shift, control, alt keys, meta keys, both left & right */
+0x2a, 0x36, 0x1d, 0x9d, 0x38, 0xb8, 0xdb, 0xdd,
+};
+
 typedef struct VirtualConsole
 {
 GtkWidget *menu_item;
@@ -161,6 +166,8 @@ typedef struct GtkDisplayState
 gboolean free_scale;
 
 bool external_pause_update;
+
+bool modifier_pressed[ARRAY_SIZE(modifier_keycode)];
 } GtkDisplayState;
 
 static GtkDisplayState *global_state;
@@ -267,6 +274,26 @@ static void gd_update_full_redraw(GtkDisplayState *s)
 gtk_widget_queue_draw_area(s->drawing_area, 0, 0, ww, wh);
 }
 
+static void gtk_release_modifiers(GtkDisplayState *s)
+{
+int i, keycode;
+
+if (!gd_on_vga(s)) {
+return;
+}
+for (i = 0; i < ARRAY_SIZE(modifier_keycode); i++) {
+keycode = modifier_keycode[i];
+if (!s->modifier_pressed[i]) {
+continue;
+}
+if (keycode & SCANCODE_GREY) {
+kbd_put_keycode(SCANCODE_EMUL0);
+}
+kbd_put_keycode(keycode | SCANCODE_UP);
+s->modifier_pressed[i] = false;
+}
+}
+
 /** DisplayState Callbacks **/
 
 static void gd_update(DisplayChangeListener *dcl,
@@ -385,8 +412,9 @@ static gboolean gd_window_key_event(GtkWidget *widget, 
GdkEventKey *key, void *o
 (key->state & IGNORE_MODIFIER_MASK) == HOTKEY_MODIFIERS) {
 handled = gtk_window_activate_key(GTK_WINDOW(widget), key);
 }
-
-if (!handled) {
+if (handled) {
+gtk_release_modifiers(s);
+} else {
 handled = gtk_window_propagate_key_event(GTK_WINDOW(widget), key);
 }
 
@@ -616,8 +644,10 @@ static gboolean gd_button_event(GtkWidget *widget, 
GdkEventButton *button,
 
 static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque)
 {
+GtkDisplayState *s = opaque;
 int gdk_keycode;
 int qemu_keycode;
+int i;
 
 gdk_keycode = key->hardware_keycode;
 
@@ -639,6 +669,12 @@ static gboolean gd_key_event(GtkWidget *widget, 
GdkEventKey *key, void *opaque)
 gdk_keycode, qemu_keycode,
 (key->type == GDK_KEY_PRESS) ? "down" : "up");
 
+for (i = 0; i < ARRAY_SIZE(modifier_keycode); i++) {
+if (qemu_keycode == modifier_keycode[i]) {
+s->modifier_pressed[i] = (key->type == GDK_KEY_PRESS);
+}
+}
+
 if (qemu_keycode & SCANCODE_GREY) {
 kbd_put_keycode(SCANCODE_EMUL0);
 }
@@ -694,6 +730,7 @@ static void gd_menu_switch_vc(GtkMenuItem *item, void 
*opaque)
 } else {
 int i;
 
+gtk_release_modifiers(s);
 for (i = 0; i < s->nb_vcs; i++) {
 if 
(gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(s->vc[i].menu_item))) {
 gtk_notebook_set_current_page(GTK_NOTEBOOK(s->notebook), i + 
1);
@@ -994,6 +1031,16 @@ static gboolean gd_leave_event(GtkWidget *widget, 
GdkEventCrossing *crossing, gp
 return TRUE;
 }
 
+static gboolean gd_focus_out_event(GtkWidget *widget,
+   GdkEventCrossing *crossing, gpointer data)
+{
+GtkDisplayState *s = data;
+
+gtk_release_modifiers(s);
+
+return TRUE;
+}
+
 /** Virtual Console Callbacks **/
 
 static int gd_vc_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
@@ -1177,6 +1224,8 @@ static void gd_connect_signals(GtkDisplayState *s)
  G_CALLBACK(gd_enter_event), s);
 g_signal_connect(s->drawing_area, "leave-notify-event",
  G_CALLBACK(gd_leave_event), s);
+g_signal_connect(s->drawing_area, "focus-out-event",
+ G_CALLBACK(gd_focus_out_event), s);
 }
 
 static void gd_create_menus(GtkDisplayState *s)
-- 
1.7.3.4



Re: [Qemu-devel] [PATCHv2 5/6] target-arm: add dump-guest-memory support

2013-03-24 Thread Peter Maydell
On 24 March 2013 17:27, Rabin Vincent  wrote:
> Enable support for the dump-guest-memory monitor command for ARM.

Hi. I'm afraid I'm not really familiar with the dump-guest-memory
command/implementation so I have some possibly dumb comments below:

> --- /dev/null
> +++ b/target-arm/arch_dump.c
> @@ -0,0 +1,61 @@
> +#include "cpu.h"
> +#include "sysemu/dump.h"
> +#include "elf.h"
> +
> +typedef struct {
> +char pad1[24];
> +uint32_t pid;
> +char pad2[44];
> +uint32_t regs[18];
> +char pad3[4];
> +} arm_elf_prstatus;

Can you point me at the spec this struct corresponds to?

> +
> +int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
> + int cpuid, void *opaque)
> +{
> +return -1;
> +}

Is there any documentation of the API we're implementing here?
(why does it require us to provide 64 bit functions that are
never used?)

> +
> +int cpu_write_elf32_note(write_core_dump_function f, CPUArchState *env,
> + int cpuid, void *opaque)
> +{
> +arm_elf_prstatus prstatus = {.pid = cpuid};
> +
> +memcpy(&(prstatus.regs), env->regs, sizeof(env->regs));
> +prstatus.regs[16] = cpsr_read(env);
> +
> +return dump_write_elf_note(ELFCLASS32, "CORE", NT_PRSTATUS,
> +   &prstatus, sizeof(prstatus),
> +   f, opaque);
> +}
> +
> +int cpu_write_elf64_qemunote(write_core_dump_function f, CPUArchState *env,
> + void *opaque)
> +{
> +return -1;
> +}
> +
> +int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
> + void *opaque)
> +{
> +return 0;
> +}

Why is it OK to implement this as a no-op? What could we
be doing here that we don't do? What are the effects?

> +
> +int cpu_get_dump_info(ArchDumpInfo *info)
> +{
> +info->d_machine = EM_ARM;
> +#ifdef TARGET_WORDS_BIGENDIAN
> +info->d_endian = ELFDATA2MSB;
> +#else
> +info->d_endian = ELFDATA2LSB;
> +#endif
> +info->d_class = ELFCLASS32;

Most of this looks like it would be suitable for a default
implementation that said "endian based on TARGET_WORDS_BIGENDIAN,
machine is ELF_MACHINE, class based on TARGET_LONG_BITS".

> +
> +return 0;
> +}
> +
> +ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
> +{
> +return nr_cpus * dump_get_note_size(ELFCLASS32, "CORE",
> +sizeof(arm_elf_prstatus));
> +}
> --
> 1.7.10.4
>

thanks
-- PMM



Re: [Qemu-devel] [PATCHv2 6/6] dump: fix memory region handling

2013-03-24 Thread Peter Maydell
On 24 March 2013 17:27, Rabin Vincent  wrote:
>  /**
> + * memory_region_get_addr: Get the address of a memory region
> + *
> + * @mr: the memory region
> + */
> +hwaddr memory_region_get_addr(MemoryRegion *mr);

I'm afraid this doesn't make sense. A MemoryRegion by itself has
no "address" -- it could be mapped into several places in several
different address maps or none at all.

-- PMM



Re: [Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Michael S. Tsirkin
On Sun, Mar 24, 2013 at 04:58:37PM +0100, Aurelien Jarno wrote:
> On Sun, Mar 24, 2013 at 11:32:30AM +, Peter Maydell wrote:
> > This patch series fixes a number of serious bugs in our emulation of
> > the PCI controller found on VersatilePB and the early Realview boards:
> >  * our interrupt mapping was totally wrong
> >  * we weren't implementing the PCI memory windows
> >  * the I/O window wasn't mapped on VersatilePB
> >  * realview mapped things at the wrong addresses
> >  * we didn't implement the controller's registers at all
> > It also updates to some reasonable approximation to QOM best practice,
> > including subclassing pci_host rather than doing everything by hand.
> > 
> > I haven't implemented support for the SMAP registers (which control
> > how the controller converts accesses made by bus-mastering PCI
> > devices into system addresses). For the moment we rely on the fact
> > that Linux always maps things 1:1. (It wouldn't be too hard to add
> > SMAP support but it requires changing QEMU's pci code to allow a
> > controller to pass in a MemoryRegion* for DMA to use instead of
> > the system address space, so I prefer to leave that for another series.)
> > 
> > Patchset tested on both versatilepb and realview, using a set of
> > Linux kernel patches written by Arnd Bergmann:
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2010-October/029040.html
> > which were in turn tested against real PB926 and PB1176 hardware.
> > 
> > 
> >  * WARNING WARNING *
> > 
> > This patchset will break any use of PCI (including the default SCSI
> > card) on versatilepb with current Linux kernels, because those kernels
> 
> Do you mean Versatile/PB and not Versatile/AB, or actually both?
> 
> > have the matching bug in interrupt mapping to old QEMU.
> 
> How is real hardware working with this bug?
> 
> > I've provided a property for enabling the old broken IRQ mapping;
> > this can be enabled with the command line option:
> >   -global versatile_pci.broken-irq-mapping=1
> > 
> > (If anybody wants to suggest a better way of handling this please do.)
> 
> Do you have a pointer to the corresponding kernel patch? Is it possible
> to get the kernel to detect if it should use the correct or the broken 
> IRQ mapping?

Alternatively, or additionally, how about detecting the correct or
the incorrect kernel and updating the mapping?
For example, maybe we could do this using the
IRQ value written into the device pci config register?

If we can't find anything, maybe add our own register
so the same qemu config can support old and new kernels?

> 
> > Peter Maydell (10):
> >   versatile_pci: Fix hardcoded tabs
> >   versatile_pci: Expose PCI I/O region on Versatile PB
> >   versatile_pci: Update to realize and instance init functions
> >   versatile_pci: Change to subclassing TYPE_PCI_HOST_BRIDGE
> >   versatile_pci: Use separate PCI I/O space rather than system I/O space
> >   versatile_pci: Put the host bridge PCI device at slot 29
> >   versatile_pci: Implement the correct PCI IRQ mapping
> >   versatile_pci: Implement the PCI controller's control registers
> >   arm/realview: Fix mapping of PCI regions
> >   versatile_pci: Expose PCI memory space to system
> > 
> >  hw/arm/realview.c|   22 +--
> >  hw/arm/versatilepb.c |   11 +-
> >  hw/versatile_pci.c   |  368 
> > --
> >  3 files changed, 344 insertions(+), 57 deletions(-)
> > 
> > -- 
> > 1.7.9.5
> > 
> > 
> > 
> 
> -- 
> Aurelien JarnoGPG: 1024D/F1BCDB73
> aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCHv2 5/6] target-arm: add dump-guest-memory support

2013-03-24 Thread Rabin Vincent
2013/3/24 Peter Maydell :
> On 24 March 2013 17:27, Rabin Vincent  wrote:
>> --- /dev/null
>> +++ b/target-arm/arch_dump.c
>> @@ -0,0 +1,61 @@
>> +#include "cpu.h"
>> +#include "sysemu/dump.h"
>> +#include "elf.h"
>> +
>> +typedef struct {
>> +char pad1[24];
>> +uint32_t pid;
>> +char pad2[44];
>> +uint32_t regs[18];
>> +char pad3[4];
>> +} arm_elf_prstatus;
>
> Can you point me at the spec this struct corresponds to?

This is elf_prstatus from the Linux kernel's
include/uapi/linux/elfcore.h, with the regset begin ARM regs in this
case.

I don't know if there's a spec.  It doesn't sound like it from the
comments in the kernel file: "This is mostly like the SVR4 structure,
but more Linuxy, with things that Linux does not support and which gdb
doesn't really use excluded."

The x86 implementation in target-i386/arch_dump.c uses the same
elf_prstatus with the x86 regs.

>
>> +
>> +int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
>> + int cpuid, void *opaque)
>> +{
>> +return -1;
>> +}
>
> Is there any documentation of the API we're implementing here?

I coudn't find any documentation.  It's only x86 that has the API
implemented.

> (why does it require us to provide 64 bit functions that are
> never used?)

I guess the API was made with x86 in mind.  I will see if the
requirement can be removed with some ifdefs in the dump.c file.

(come to think of it, I guess this ARM code will need to use ELFCLASS64
 when we have physical memory > 4GiB (LPAE))

>> +int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
>> + void *opaque)
>> +{
>> +return 0;
>> +}
>
> Why is it OK to implement this as a no-op? What could we
> be doing here that we don't do? What are the effects?

This is supposed to be used to add some additional information about the
CPU state in an ELF note in a QEMU-specific structure, like the
QEMUCPUState in target-i386/arm-state.c.  It's only useful to implement
this if someone sees a need to add any such information.

>
>> +
>> +int cpu_get_dump_info(ArchDumpInfo *info)
>> +{
>> +info->d_machine = EM_ARM;
>> +#ifdef TARGET_WORDS_BIGENDIAN
>> +info->d_endian = ELFDATA2MSB;
>> +#else
>> +info->d_endian = ELFDATA2LSB;
>> +#endif
>> +info->d_class = ELFCLASS32;
>
> Most of this looks like it would be suitable for a default
> implementation that said "endian based on TARGET_WORDS_BIGENDIAN,
> machine is ELF_MACHINE, class based on TARGET_LONG_BITS".

I will see if this can be moved into the generic dump.c.



Re: [Qemu-devel] [PATCHv2 6/6] dump: fix memory region handling

2013-03-24 Thread Rabin Vincent
2013/3/24 Peter Maydell :
> On 24 March 2013 17:27, Rabin Vincent  wrote:
>>  /**
>> + * memory_region_get_addr: Get the address of a memory region
>> + *
>> + * @mr: the memory region
>> + */
>> +hwaddr memory_region_get_addr(MemoryRegion *mr);
>
> I'm afraid this doesn't make sense. A MemoryRegion by itself has
> no "address" -- it could be mapped into several places in several
> different address maps or none at all.

OK.  Do you mean that such a function can be used internally to the dump
code where it gets the MemoryRegion only from the RAMBlock.mr or do you
mean the dump code also shouldn't be doing it that way?

If you mean the latter, could you please suggest an alternative way to
handle this?  The problem is that the dump code assumes that
RAMBlock.offset provides the physical address, and this appears to not
be the case.  For example, with a dump generated from vexpress I get
these Program Headers in the dump:

  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  NOTE   0xf4 0x 0x 0x002a0 0x002a0 0
  LOAD   0x000394 0x 0x 0x800 0x800 0
  LOAD   0x8000394 0x 0x0800 0x400 0x400 0
  LOAD   0xc000394 0x 0x0c00 0x400 0x400 0
  LOAD   0x1394 0x 0x1000 0x200 0x200 0
  LOAD   0x12000394 0x 0x1200 0x80 0x80 0

The physical addresses are completely wrong, and with the patch I get
the right ones:

  Type   Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  NOTE   0xf4 0x 0x 0x002a0 0x002a0 0
  LOAD   0x8000394 0x 0x4000 0x400 0x400 0
  LOAD   0xc000394 0x 0x4400 0x400 0x400 0
  LOAD   0x1394 0x 0x4800 0x200 0x200 0
  LOAD   0x12000394 0x 0x4c00 0x80 0x80 0
  LOAD   0x000394 0x 0x6000 0x800 0x800 0

Thanks.



Re: [Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Aurelien Jarno
On Sun, Mar 24, 2013 at 04:49:37PM +, Peter Maydell wrote:
> On 24 March 2013 15:58, Aurelien Jarno  wrote:
> > On Sun, Mar 24, 2013 at 11:32:30AM +, Peter Maydell wrote:
> >> This patch series fixes a number of serious bugs in our emulation of
> >> the PCI controller found on VersatilePB and the early Realview boards:
> 
> >> Patchset tested on both versatilepb and realview, using a set of
> >> Linux kernel patches written by Arnd Bergmann:
> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-October/029040.html
> >> which were in turn tested against real PB926 and PB1176 hardware.
> >>
> >>
> >>  * WARNING WARNING *
> >>
> >> This patchset will break any use of PCI (including the default SCSI
> >> card) on versatilepb with current Linux kernels, because those kernels
> >
> > Do you mean Versatile/PB and not Versatile/AB, or actually both?
> 
> I mean PB. The AB doesn't have a PCI controller (though we incorrectly
> model it as having one; I suppose we could patch QEMU to stop doing
> that).
> 
> >> have the matching bug in interrupt mapping to old QEMU.
> >
> > How is real hardware working with this bug?
> 
> It doesn't. Unless you apply Arnd's patches, PCI support
> on real hardware is flat out broken. (This was never noticed
> because (a) real hardware is getting rare by now and (b) the
> PCI backplane is even rarer.)
> 
> >> I've provided a property for enabling the old broken IRQ mapping;
> >> this can be enabled with the command line option:
> >>   -global versatile_pci.broken-irq-mapping=1
> >>
> >> (If anybody wants to suggest a better way of handling this please do.)
> >
> > Do you have a pointer to the corresponding kernel patch? Is it possible
> > to get the kernel to detect if it should use the correct or the broken
> > IRQ mapping?
> 
> I linked to the kernel patches above. It might I guess be possible
> to identify a fixed QEMU (if nothing else we could put something
> into QEMU that was identifiable); that still leaves existing kernels
> breaking, though.
> 

Now that stable kernel releases work well and that most distribution are
basing their releases on longterm kernels, it might be possible to get
the fix included there, and from there in the distributions. I think
it's something that would be acceptable, provided it is below the 100
lines limit including context.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCHv2 6/6] dump: fix memory region handling

2013-03-24 Thread Peter Maydell
On 24 March 2013 19:35, Rabin Vincent  wrote:
> 2013/3/24 Peter Maydell :
>> On 24 March 2013 17:27, Rabin Vincent  wrote:
>>>  /**
>>> + * memory_region_get_addr: Get the address of a memory region
>>> + *
>>> + * @mr: the memory region
>>> + */
>>> +hwaddr memory_region_get_addr(MemoryRegion *mr);
>>
>> I'm afraid this doesn't make sense. A MemoryRegion by itself has
>> no "address" -- it could be mapped into several places in several
>> different address maps or none at all.
>
> OK.  Do you mean that such a function can be used internally to the dump
> code where it gets the MemoryRegion only from the RAMBlock.mr or do you
> mean the dump code also shouldn't be doing it that way?

I mean that the dump code is wrong if it's trying to ask "what
is the address of this MemoryRegion (or RAMBlock)" at all. That
question doesn't have a well defined single answer.

> If you mean the latter, could you please suggest an alternative way to
> handle this?  The problem is that the dump code assumes that
> RAMBlock.offset provides the physical address, and this appears to not
> be the case.

That is also wrong, both for the reason you state and also because
ramblocks can be aliased into multiple physical addresses too.
I suspect you need to change anything that's trying to iterate
through memory with "for each block in ram_list" to instead actually
iterate through the system address space and say "if this is RAM
then...". For instance with the vexpress-a9 you presumably want
a LOAD section for both physaddr 0 (the alias) and physaddr
0x6000 (the usual address).

-- PMM



Re: [Qemu-devel] [PATCHv2 5/6] target-arm: add dump-guest-memory support

2013-03-24 Thread Peter Maydell
On 24 March 2013 19:26, Rabin Vincent  wrote:
> 2013/3/24 Peter Maydell :
>> On 24 March 2013 17:27, Rabin Vincent  wrote:

So I guess I should prefix this email by saying that quite
a bit of it is really comments on the existing dump code
rather than the ARM related support you're adding here. I
appreciate that it's annoying to get asked to fix up a bunch
of existing code just because you happened to be the next
person to want to add use case 2 to it. So you should feel free
to push back a bit if any of this seems too burdensome or
unnecessary.

(I do think that the memory region stuff in the other patch
does need fixing though, because (a) vexpress really does
map RAM in two places in the physical address map and (b) we
need to be careful about what we allow to be added to the
memory API, so it remains coherent. So if you only fix up one
thing it should be that.)

>>> --- /dev/null
>>> +++ b/target-arm/arch_dump.c
>>> @@ -0,0 +1,61 @@
>>> +#include "cpu.h"
>>> +#include "sysemu/dump.h"
>>> +#include "elf.h"
>>> +
>>> +typedef struct {
>>> +char pad1[24];
>>> +uint32_t pid;
>>> +char pad2[44];
>>> +uint32_t regs[18];
>>> +char pad3[4];
>>> +} arm_elf_prstatus;
>>
>> Can you point me at the spec this struct corresponds to?
>
> This is elf_prstatus from the Linux kernel's
> include/uapi/linux/elfcore.h, with the regset begin ARM regs in this
> case.

They don't look very similar to me -- this one has a lot of
pad fields the elf_prstatus doesn't. Also, if the kernel's
struct is a standard one with a cpu-specific regset field,
why isn't QEMU also using a standard struct with a cpu-specific
regset field?

(it seems a bit bogus that we aren't saving the floating point
registers too, but it looks like Linux doesn't do that either,
so I guess there's nowhere in the core file for it.)

> I don't know if there's a spec.  It doesn't sound like it from the
> comments in the kernel file: "This is mostly like the SVR4 structure,
> but more Linuxy, with things that Linux does not support and which gdb
> doesn't really use excluded."
>
> The x86 implementation in target-i386/arch_dump.c uses the same
> elf_prstatus with the x86 regs.
>
>>
>>> +
>>> +int cpu_write_elf64_note(write_core_dump_function f, CPUArchState *env,
>>> + int cpuid, void *opaque)
>>> +{
>>> +return -1;
>>> +}
>>
>> Is there any documentation of the API we're implementing here?
>
> I coudn't find any documentation.  It's only x86 that has the API
> implemented.

It would be nice if somebody who understood it could add some
doc comments to the header file.

>> (why does it require us to provide 64 bit functions that are
>> never used?)
>
> I guess the API was made with x86 in mind.  I will see if the
> requirement can be removed with some ifdefs in the dump.c file.
>
> (come to think of it, I guess this ARM code will need to use ELFCLASS64
>  when we have physical memory > 4GiB (LPAE))

It would be good to check whether that is correct -- mostly core
files are for dumps of a virtual address space so I don't know
whether gdb/etc would understand an ARM corefile which claimed
ELFCLASS64. (I mean understand it as an AArch32 corefile rather
than AArch64.)

>>> +int cpu_write_elf32_qemunote(write_core_dump_function f, CPUArchState *env,
>>> + void *opaque)
>>> +{
>>> +return 0;
>>> +}
>>
>> Why is it OK to implement this as a no-op? What could we
>> be doing here that we don't do? What are the effects?
>
> This is supposed to be used to add some additional information about the
> CPU state in an ELF note in a QEMU-specific structure, like the
> QEMUCPUState in target-i386/arm-state.c.  It's only useful to implement
> this if someone sees a need to add any such information.

OK.

>>
>>> +
>>> +int cpu_get_dump_info(ArchDumpInfo *info)
>>> +{
>>> +info->d_machine = EM_ARM;
>>> +#ifdef TARGET_WORDS_BIGENDIAN
>>> +info->d_endian = ELFDATA2MSB;
>>> +#else
>>> +info->d_endian = ELFDATA2LSB;
>>> +#endif
>>> +info->d_class = ELFCLASS32;
>>
>> Most of this looks like it would be suitable for a default
>> implementation that said "endian based on TARGET_WORDS_BIGENDIAN,
>> machine is ELF_MACHINE, class based on TARGET_LONG_BITS".
>
> I will see if this can be moved into the generic dump.c.

thanks
-- PMM



Re: [Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Peter Maydell
On 24 March 2013 19:17, Michael S. Tsirkin  wrote:
> On Sun, Mar 24, 2013 at 04:58:37PM +0100, Aurelien Jarno wrote:
>> On Sun, Mar 24, 2013 at 11:32:30AM +, Peter Maydell wrote:
>> > I've provided a property for enabling the old broken IRQ mapping;
>> > this can be enabled with the command line option:
>> >   -global versatile_pci.broken-irq-mapping=1
>> >
>> > (If anybody wants to suggest a better way of handling this please do.)
>>
>> Do you have a pointer to the corresponding kernel patch? Is it possible
>> to get the kernel to detect if it should use the correct or the broken
>> IRQ mapping?
>
> Alternatively, or additionally, how about detecting the correct or
> the incorrect kernel and updating the mapping?
> For example, maybe we could do this using the
> IRQ value written into the device pci config register?

Yeah, ideally being able to detect the buggy kernel would be good;
I can't see anything at the controller level that would do though,
and I don't really know enough about PCI to know about generic
PCI stuff that would work. (Why would the OS need to tell the
device anything about its IRQ if it's hardwired?)

> If we can't find anything, maybe add our own register
> so the same qemu config can support old and new kernels?

Then the new kernel wouldn't work on real hardware...

-- PMM



Re: [Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Arnd Bergmann
On Sunday 24 March 2013, Peter Maydell wrote:
> Yeah, ideally being able to detect the buggy kernel would be good;
> I can't see anything at the controller level that would do though,
> and I don't really know enough about PCI to know about generic
> PCI stuff that would work. (Why would the OS need to tell the
> device anything about its IRQ if it's hardwired?)

I think it actually does on versatile and other platforms on which
the kernel probes the PCI bus itself, rather than relying on firmware
to have resources assigned in advance.

IIRC, the PCI_INTERRUPT_LINE pci config space byte (0x3c) is purely
informational and used as a way to communicate the interrupt number
from the bus scan code (assumed to be a PC BIOS in the PCI spec,
but drivers/pci/setup-irq.c in case of versatile+linux) to a device
driver.

So the kernel should actually write the proper interrupt number in
there. In future kernels, this may not necessarily be the hardware
number, but today it is. Can you try out what the kernel writes into
that register in qemu, with and without my patches?

I would expect the numbers to be (64+27) to (64+30), since we
linearize the interrupt numbers so that VIC gets 32 through
63 and SIC gets 64 through 95.

Arnd



Re: [Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Peter Maydell
On 24 March 2013 21:16, Arnd Bergmann  wrote:
> On Sunday 24 March 2013, Peter Maydell wrote:
>> Yeah, ideally being able to detect the buggy kernel would be good;
>> I can't see anything at the controller level that would do though,
>> and I don't really know enough about PCI to know about generic
>> PCI stuff that would work. (Why would the OS need to tell the
>> device anything about its IRQ if it's hardwired?)
>
> I think it actually does on versatile and other platforms on which
> the kernel probes the PCI bus itself, rather than relying on firmware
> to have resources assigned in advance.
>
> IIRC, the PCI_INTERRUPT_LINE pci config space byte (0x3c) is purely
> informational and used as a way to communicate the interrupt number
> from the bus scan code (assumed to be a PC BIOS in the PCI spec,
> but drivers/pci/setup-irq.c in case of versatile+linux) to a device
> driver.
>
> So the kernel should actually write the proper interrupt number in
> there. In future kernels, this may not necessarily be the hardware
> number, but today it is. Can you try out what the kernel writes into
> that register in qemu, with and without my patches?

OK, I'll give that a go tomorrow.

While you're here, do you know what the point of the PCI_SELFID
register is? The h/w docs are clear that the OS has to write
the slot number of the board itself in to this register, but
again I don't see why the OS has to tell the hardware something
it already knows. On QEMU I just implemented this register as
a no-op and everything seems to still function...

thanks
-- PMM



Re: [Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Michael S. Tsirkin
On Sun, Mar 24, 2013 at 08:53:33PM +, Peter Maydell wrote:
> On 24 March 2013 19:17, Michael S. Tsirkin  wrote:
> > On Sun, Mar 24, 2013 at 04:58:37PM +0100, Aurelien Jarno wrote:
> >> On Sun, Mar 24, 2013 at 11:32:30AM +, Peter Maydell wrote:
> >> > I've provided a property for enabling the old broken IRQ mapping;
> >> > this can be enabled with the command line option:
> >> >   -global versatile_pci.broken-irq-mapping=1
> >> >
> >> > (If anybody wants to suggest a better way of handling this please do.)
> >>
> >> Do you have a pointer to the corresponding kernel patch? Is it possible
> >> to get the kernel to detect if it should use the correct or the broken
> >> IRQ mapping?
> >
> > Alternatively, or additionally, how about detecting the correct or
> > the incorrect kernel and updating the mapping?
> > For example, maybe we could do this using the
> > IRQ value written into the device pci config register?
> 
> Yeah, ideally being able to detect the buggy kernel would be good;
> I can't see anything at the controller level that would do though,
> and I don't really know enough about PCI to know about generic
> PCI stuff that would work. (Why would the OS need to tell the
> device anything about its IRQ if it's hardwired?)

Each pci device has a bit of memory where the OS stores
the IRQ#. I think it was invented as a simple way to pass
data from BIOS to the OS on a PC.
There's no special reason to store it on the device
but everyone does it.

> > If we can't find anything, maybe add our own register
> > so the same qemu config can support old and new kernels?
> 
> Then the new kernel wouldn't work on real hardware...
> 
> -- PMM

Not if our register is read-only in real hardware.

-- 
MST



Re: [Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Michael S. Tsirkin
On Sun, Mar 24, 2013 at 09:16:28PM +, Arnd Bergmann wrote:
> On Sunday 24 March 2013, Peter Maydell wrote:
> > Yeah, ideally being able to detect the buggy kernel would be good;
> > I can't see anything at the controller level that would do though,
> > and I don't really know enough about PCI to know about generic
> > PCI stuff that would work. (Why would the OS need to tell the
> > device anything about its IRQ if it's hardwired?)
> 
> I think it actually does on versatile and other platforms on which
> the kernel probes the PCI bus itself, rather than relying on firmware
> to have resources assigned in advance.
> 
> IIRC, the PCI_INTERRUPT_LINE pci config space byte (0x3c) is purely
> informational and used as a way to communicate the interrupt number
> from the bus scan code (assumed to be a PC BIOS in the PCI spec,
> but drivers/pci/setup-irq.c in case of versatile+linux) to a device
> driver.
> 
> So the kernel should actually write the proper interrupt number in
> there. In future kernels, this may not necessarily be the hardware
> number, but today it is.

For future kernels, let's build in some hook that let
qemu detect a non broken guest. How about writing
some magic value into revision ID or some other
readonly field?

> Can you try out what the kernel writes into
> that register in qemu, with and without my patches?
> 
> I would expect the numbers to be (64+27) to (64+30), since we
> linearize the interrupt numbers so that VIC gets 32 through
> 63 and SIC gets 64 through 95.
> 
>   Arnd



Re: [Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Arnd Bergmann
On Sunday 24 March 2013, Peter Maydell wrote:
> OK, I'll give that a go tomorrow.
> 
> While you're here, do you know what the point of the PCI_SELFID
> register is? The h/w docs are clear that the OS has to write
> the slot number of the board itself in to this register, but
> again I don't see why the OS has to tell the hardware something
> it already knows. On QEMU I just implemented this register as
> a no-op and everything seems to still function...
> 

I don't know really, but I think it has something to do with the
versatile board being plugged into a backplane that has multiple
slots. Apparently there is something the hardware cannot figure
out itself, e.g. whether to route the interrupts in or out of the
slot.

Arnd



Re: [Qemu-devel] [PATCH 00/10] Fix versatile_pci (and break versatilepb linux guests!)

2013-03-24 Thread Arnd Bergmann
On Sunday 24 March 2013, Michael S. Tsirkin wrote:
> For future kernels, let's build in some hook that let
> qemu detect a non broken guest. How about writing
> some magic value into revision ID or some other
> readonly field?

Yes, makes sense.



Re: [Qemu-devel] [PATCH] hw/arm_mptimer: Save the timer state

2013-03-24 Thread Peter Crosthwaite
Hi Peter,

On Sat, Mar 23, 2013 at 1:49 AM, Peter Maydell  wrote:
> Add a missing VMSTATE_TIMER() entry to the arm_mptimer vmstate
> description; this omission meant that we would probably hang on reload
> when the timer failed to fire.
>

I know load-save does not work for Zynq Linux. This will definately
help get us closer to a working state. Might have another go to see if
it gets any further with this. Thanks.

> Signed-off-by: Peter Maydell 

Reviewed-by: Peter Crosthwaite 

> ---
>  hw/arm_mptimer.c |5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm_mptimer.c b/hw/arm_mptimer.c
> index f59a9f1..317f5e4 100644
> --- a/hw/arm_mptimer.c
> +++ b/hw/arm_mptimer.c
> @@ -253,14 +253,15 @@ static int arm_mptimer_init(SysBusDevice *dev)
>
>  static const VMStateDescription vmstate_timerblock = {
>  .name = "arm_mptimer_timerblock",
> -.version_id = 1,
> -.minimum_version_id = 1,
> +.version_id = 2,
> +.minimum_version_id = 2,
>  .fields = (VMStateField[]) {
>  VMSTATE_UINT32(count, TimerBlock),
>  VMSTATE_UINT32(load, TimerBlock),
>  VMSTATE_UINT32(control, TimerBlock),
>  VMSTATE_UINT32(status, TimerBlock),
>  VMSTATE_INT64(tick, TimerBlock),
> +VMSTATE_TIMER(timer, TimerBlock),
>  VMSTATE_END_OF_LIST()
>  }
>  };
> --
> 1.7.9.5
>
>



Re: [Qemu-devel] [PATCH 02/10] versatile_pci: Expose PCI I/O region on Versatile PB

2013-03-24 Thread Peter Crosthwaite
Hi Peter,

On Sun, Mar 24, 2013 at 9:32 PM, Peter Maydell  wrote:
> Comments in the QEMU source code claim that the version of the PCI
> controller on the VersatilePB board doesn't support the PCI I/O
> region, but this is incorrect; expose that region, map it in the
> correct location, and drop the misleading comments.
>
> This change removes the only currently implemented difference
> between the realview-pci and versatile-pci models; however there
> are other differences in not-yet-implemented functionality, so we
> retain the distinction between the two device types.
>
> Signed-off-by: Peter Maydell 
> ---
>  hw/arm/versatilepb.c |3 +--
>  hw/versatile_pci.c   |8 +++-
>  2 files changed, 4 insertions(+), 7 deletions(-)
>
> diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c
> index baaa265..0d08d15 100644
> --- a/hw/arm/versatilepb.c
> +++ b/hw/arm/versatilepb.c
> @@ -226,14 +226,13 @@ static void versatile_init(QEMUMachineInitArgs *args, 
> int board_id)
>  qdev_init_nofail(dev);
>  sysbus_mmio_map(busdev, 0, 0x4100); /* PCI self-config */
>  sysbus_mmio_map(busdev, 1, 0x4200); /* PCI config */
> +sysbus_mmio_map(busdev, 2, 0x4300); /* PCI I/O */
>  sysbus_connect_irq(busdev, 0, sic[27]);
>  sysbus_connect_irq(busdev, 1, sic[28]);
>  sysbus_connect_irq(busdev, 2, sic[29]);
>  sysbus_connect_irq(busdev, 3, sic[30]);
>  pci_bus = (PCIBus *)qdev_get_child_bus(dev, "pci");
>
> -/* The Versatile PCI bridge does not provide access to PCI IO space,
> -   so many of the qemu PCI devices are not useable.  */
>  for(n = 0; n < nb_nics; n++) {
>  nd = &nd_table[n];
>
> diff --git a/hw/versatile_pci.c b/hw/versatile_pci.c
> index 16ce5d1..1312f46 100644
> --- a/hw/versatile_pci.c
> +++ b/hw/versatile_pci.c
> @@ -77,7 +77,7 @@ static int pci_vpb_init(SysBusDevice *dev)
>  /* Our memory regions are:
>   * 0 : PCI self config window
>   * 1 : PCI config window
> - * 2 : PCI IO window (realview_pci only)
> + * 2 : PCI IO window
>   */
>  memory_region_init_io(&s->mem_config, &pci_vpb_config_ops, bus,
>"pci-vpb-selfconfig", 0x100);
> @@ -85,10 +85,8 @@ static int pci_vpb_init(SysBusDevice *dev)
>  memory_region_init_io(&s->mem_config2, &pci_vpb_config_ops, bus,
>"pci-vpb-config", 0x100);
>  sysbus_init_mmio(dev, &s->mem_config2);
> -if (s->realview) {

This is the one and only usage of ->realview. I wonder if this
argument is flawed - in real hardware is there any functional
difference between realview and versatile PCI requiring the level of
heirachy defined here? Could we take the oppurtunity to knock out this
realview flag and the "realview_pci" class altogether and save on all
the QOM boilerplate for the now functionally identical subclass?

Regards,
Peter

> -isa_mmio_setup(&s->isa, 0x010);
> -sysbus_init_mmio(dev, &s->isa);
> -}
> +isa_mmio_setup(&s->isa, 0x010);
> +sysbus_init_mmio(dev, &s->isa);
>
>  pci_create_simple(bus, -1, "versatile_pci_host");
>  return 0;
> --
> 1.7.9.5
>
>



Re: [Qemu-devel] [RFC qemu PATCH] only writing out the last byte of MAC makes it have effect

2013-03-24 Thread Amos Kong
On Fri, Mar 22, 2013 at 10:45:09AM +1030, Rusty Russell wrote:
> "Michael S. Tsirkin"  writes:
> > On Thu, Mar 21, 2013 at 02:44:50PM +0800, Amos Kong wrote:
> >> The lengcy guests don't have mac programming command, we don't know when
> >> it's safe to use MAC. This patch changed qemu to makes MAC change effect
> >> when the last byte of MAC is written to config space.
> >> 
> >> MAC address takes first 6 bytes of config space of virtio-net, the addr
> >> is 5 when the last byte is written in virtio_config_writeb().
> >> 
> >> MAC change will effect when n->mac is updated in virtio_net_set_config().
> >> 
> >> Signed-off-by: Amos Kong 
> >
> > Let's see what Rusty says about the spec change.
> 
> Implementation notes like this belong as a footnote, eg:
> 
> For older systems, it is recommended and typical that the device
> write byte 5 of the mac address last, so devices can use that as
> a trigger to commit the mac address change.
> 
> Now, is this a real, or theoretical issue?  Have we seen this problem in
> practice, or should we continue to ignore it?

Hi Rusty, Michael

I didn't touch any problem. MST, and you?

In Linux guest, we should disable the interface before changing mac address.
 ifconfig eth0 down
 ifconfig eth0 hw ether 10:12:13:14:15:16
 ifconfig eth0 up
 
In Windows 7 guest, after changing mac address in register table,
re-enabling interface to make it effect.
 reg add HKLM 
SYSTEM\CurrentControlSet\Control\Class\{4D36E972-E325-11CE-BFC1-08002BE10318}\0001]
 /v NetworkAddress /d 0123456789AB
 netsh interface set interface "Local Area Connection" DISABLED
 netsh interface set interface "Local Area Connection" ENABLED


So when we change the mac address, guest os always disable interface
to receive all packages. It seems the theoretical issue doesn't exist?

> Cheers,
> Rusty.
> 

-- 
Amos.



Re: [Qemu-devel] [PATCH V12 3/4] Use QemuOpts support in block layer

2013-03-24 Thread Dong Xu Wang
On Thu, Mar 21, 2013 at 11:31 PM, Markus Armbruster  wrote:
> Dong Xu Wang  writes:
>
>> This patch will use QemuOpts related functions in block layer, add
>> a member bdrv_create_opts to BlockDriver struct, it will return
>> a QemuOptsList pointer, which includes the image format's create
>> options.
>>
>> And create options's primary consumer is block creating related
>> functions, so modify them together.
>>
>> Signed-off-by: Dong Xu Wang 
>> ---
>>
>> v11->v12:
>> 1) create functions, such as qemu_opt_get_del  and qemu_opt_replace_set.
>> These functions works like origin code.
>> 2) use QEMU_OPT_SIZE, not QEMU_OPT_NUMBER.
>> 3) in bdrv_create, if opts is NULL, will create an empty one, so can
>> discard if(opts) code safely.
>>
>> v10->v11:
>> 1) qed.h move QED_DEFAULT_CLUSTER_SIZE from enum to macro, or
>> qemu_opts_print produce un-expanded cluster_size.
>> 2) In qcow2.c and qcow.c, bdrv_create_file(filename, NULL), NULL ->
>> opts,
>> or while using protocol, there will be an error.
>>
>> v8->v9:
>> 1) add qemu_ prefix to gluster_create_opts.
>> 2) fix bug: bdrv_gluster_unix and bdrv_gluster_rdma should also be
>>converted.
>>
>> v7->v8:
>> 1) rebase to upstream source tree.
>> 2) add gluster.c, raw-win32.c, and rbd.c.
>>
>> v6->v7:
>> 1) use osdep.h:stringify(), not redefining new macro.
>> 2) preserve TODO comment.
>> 3) fix typo. BLOCK_OPT_ENCRYPT->BLOCK_OPT_STATIC.
>> 4) initialize disk_type even when opts is NULL.
>>
>> v5->v6:
>> 1) judge if opts == NULL in block layer create functions.
>> 2) use bdrv_create_file(filename, NULL) in qcow_create and cow_create
>> funtion.
>> 3) made more readable while using qemu_opt_get_number.
>>
>>
>>  block.c   |  91 
>>  block/cow.c   |  46 ++--
>>  block/gluster.c   |  37 +-
>>  block/iscsi.c |   8 +--
>>  block/qcow.c  |  61 
>>  block/qcow2.c | 173 
>> +++---
>>  block/qed.c   |  86 +++
>>  block/qed.h   |   2 +-
>>  block/raw-posix.c |  59 +++-
>>  block/raw-win32.c |  31 +
>>  block/raw.c   |  30 
>>  block/rbd.c   |  62 -
>>  block/sheepdog.c  |  75 ++--
>>  block/vdi.c   |  70 +--
>>  block/vmdk.c  |  90 
>>  block/vpc.c   |  57 +++
>>  block/vvfat.c |  11 +--
>>  include/block/block.h |   4 +-
>>  include/block/block_int.h |   6 +-
>>  include/qemu/option.h |  13 +++-
>>  qemu-img.c|  61 
>>  util/qemu-option.c|  93 +++--
>>  22 files changed, 613 insertions(+), 553 deletions(-)
>
> *Ouch*
>
> Any chance to split this patch up some?  Its size makes it really hard
> to review...
>
I will split this patch into some small patches in next version.

>> diff --git a/block.c b/block.c
>> index 4582961..975c3d8 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -357,7 +357,7 @@ BlockDriver *bdrv_find_whitelisted_format(const char 
>> *format_name)
>>  typedef struct CreateCo {
>>  BlockDriver *drv;
>>  char *filename;
>> -QEMUOptionParameter *options;
>> +QemuOpts *opts;
>>  int ret;
>>  } CreateCo;
>>
>> @@ -366,11 +366,11 @@ static void coroutine_fn bdrv_create_co_entry(void 
>> *opaque)
>>  CreateCo *cco = opaque;
>>  assert(cco->drv);
>>
>> -cco->ret = cco->drv->bdrv_create(cco->filename, cco->options);
>> +cco->ret = cco->drv->bdrv_create(cco->filename, cco->opts);
>>  }
>>
>>  int bdrv_create(BlockDriver *drv, const char* filename,
>> -QEMUOptionParameter *options)
>> +QemuOpts *opts)
>
> Since you touch this anyway, consider unbreaking the line:
>
> int bdrv_create(BlockDriver *drv, const char* filename, QemuOpts *opts)
>
Okay.
>>  {
>>  int ret;
>>
>> @@ -378,7 +378,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
>>  CreateCo cco = {
>>  .drv = drv,
>>  .filename = g_strdup(filename),
>> -.options = options,
>> +.opts = opts ?: qemu_opts_create_nofail(drv->bdrv_create_opts),
>>  .ret = NOT_DONE,
>>  };
>>
>
> As discussed during review of v11, this avoids passing null opts to the
> bdrv_create() method.  Good.
>
>> @@ -405,7 +405,7 @@ out:
>out:
>g_free(cco.filename);
>>  return ret;
>>  }
>
> I suspect you need
>
> if (!opts) {
> qemu_opts_del(cco->opts);
> }
>
> to avoid leaking the empty cco->opts you create in the previous hunk.
>
Okay.
>>
>> -int bdrv_create_file(const char* filename, QEMUOptionParameter *options)
>> +int bdrv_create_file(const char *filename, QemuOpts *opts)
>>  {
>>  BlockDriver *drv;
>>
>> @@ -414,7 +414,7 @@ int bdrv_create_file(const char* filename, 
>> QEMUOptionParameter *options)
>>  return

Re: [Qemu-devel] [RFC qemu PATCH] only writing out the last byte of MAC makes it have effect

2013-03-24 Thread Michael S. Tsirkin
On Mon, Mar 25, 2013 at 10:23:57AM +0800, Amos Kong wrote:
> On Fri, Mar 22, 2013 at 10:45:09AM +1030, Rusty Russell wrote:
> > "Michael S. Tsirkin"  writes:
> > > On Thu, Mar 21, 2013 at 02:44:50PM +0800, Amos Kong wrote:
> > >> The lengcy guests don't have mac programming command, we don't know when
> > >> it's safe to use MAC. This patch changed qemu to makes MAC change effect
> > >> when the last byte of MAC is written to config space.
> > >> 
> > >> MAC address takes first 6 bytes of config space of virtio-net, the addr
> > >> is 5 when the last byte is written in virtio_config_writeb().
> > >> 
> > >> MAC change will effect when n->mac is updated in virtio_net_set_config().
> > >> 
> > >> Signed-off-by: Amos Kong 
> > >
> > > Let's see what Rusty says about the spec change.
> > 
> > Implementation notes like this belong as a footnote, eg:
> > 
> > For older systems, it is recommended and typical that the device
> > write byte 5 of the mac address last, so devices can use that as
> > a trigger to commit the mac address change.
> > 
> > Now, is this a real, or theoretical issue?  Have we seen this problem in
> > practice, or should we continue to ignore it?
> 
> Hi Rusty, Michael
> 
> I didn't touch any problem. MST, and you?
> 
> In Linux guest, we should disable the interface before changing mac address.
>  ifconfig eth0 down
>  ifconfig eth0 hw ether 10:12:13:14:15:16
>  ifconfig eth0 up
>  
> In Windows 7 guest, after changing mac address in register table,
> re-enabling interface to make it effect.
>  reg add HKLM 
> SYSTEM\CurrentControlSet\Control\Class\{4D36E972-E325-11CE-BFC1-08002BE10318}\0001]
>  /v NetworkAddress /d 0123456789AB
>  netsh interface set interface "Local Area Connection" DISABLED
>  netsh interface set interface "Local Area Connection" ENABLED
> 
> 
> So when we change the mac address, guest os always disable interface
> to receive all packages. It seems the theoretical issue doesn't exist?
> 
> > Cheers,
> > Rusty.
> > 
> 
> -- 
>   Amos.

Nope. Looks like no spec change is necessary. We already say it's
not atomic and it looks like guests expect exactly that
and disable link to prevent strange issues.

-- 
MST