[Qemu-devel] [PATCH] configure: show debug-info option in --help output
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)
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
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)
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
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
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
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.
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
$ ./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
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
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
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
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
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
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
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
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
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!)
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
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
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
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
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
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
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
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
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
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
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
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
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 )
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 )
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
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
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
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
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
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
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!)
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!)
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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!)
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/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/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!)
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
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
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!)
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!)
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!)
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!)
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!)
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!)
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!)
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
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
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
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
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
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