[Qemu-devel] [PATCH arm-devs v1 14/15] xilinx_spips: lqspi: Push more data to tx-fifo
Do 16 words per fifo flush. Increases performance and decreases debug verbosity. This data depth has no real hardware analogue, so just go with something that has reasonable performance. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/xilinx_spips.c | 11 +++ 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/hw/xilinx_spips.c b/hw/xilinx_spips.c index 5ac0f64..32d8db8 100644 --- a/hw/xilinx_spips.c +++ b/hw/xilinx_spips.c @@ -571,11 +571,14 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size) DB_PRINT_L(0, starting QSPI data read\n); -for (i = 0; i LQSPI_CACHE_SIZE / 4; ++i) { -tx_data_bytes(s, 0, 4); +while (cache_entry LQSPI_CACHE_SIZE / 4) { +for (i = 0; i 16; ++i) { +tx_data_bytes(s, 0, 4); +} xilinx_spips_flush_txfifo(s); -rx_data_bytes(s, q-lqspi_buf[cache_entry], 4); -cache_entry++; +for (i = 0; i 16; ++i) { +rx_data_bytes(s, q-lqspi_buf[cache_entry++], 4); +} } s-regs[R_CONFIG] |= CS; -- 1.7.0.4
[Qemu-devel] [PATCH arm-devs v1 15/15] xilinx_spips: lqspi: Fix byte/misaligned access
The LQSPI bus attachment supports byte/halfword and misaligned accesses. Fixed. Refactored the LQSPI cache to be byte-wise instead of word wise accordingly. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/xilinx_spips.c | 31 +-- 1 files changed, 17 insertions(+), 14 deletions(-) diff --git a/hw/xilinx_spips.c b/hw/xilinx_spips.c index 32d8db8..cb45a9c 100644 --- a/hw/xilinx_spips.c +++ b/hw/xilinx_spips.c @@ -160,7 +160,7 @@ typedef struct { typedef struct { XilinxSPIPS parent_obj; -uint32_t lqspi_buf[LQSPI_CACHE_SIZE]; +uint8_t lqspi_buf[LQSPI_CACHE_SIZE]; hwaddr lqspi_cached_addr; } XilinxQSPIPS; @@ -366,14 +366,12 @@ static void xilinx_spips_flush_txfifo(XilinxSPIPS *s) } } -static inline void rx_data_bytes(XilinxSPIPS *s, uint32_t *value, int max) +static inline void rx_data_bytes(XilinxSPIPS *s, uint8_t *value, int max) { int i; -*value = 0; for (i = 0; i max !fifo8_is_empty(s-rx_fifo); ++i) { -uint32_t next = fifo8_pop(s-rx_fifo) 0xFF; -*value |= next 8 * (s-regs[R_CONFIG] ENDIAN ? 3-i : i); +value[i] = fifo8_pop(s-rx_fifo); } } @@ -383,6 +381,7 @@ static uint64_t xilinx_spips_read(void *opaque, hwaddr addr, XilinxSPIPS *s = opaque; uint32_t mask = ~0; uint32_t ret; +uint8_t rx_buf[4]; addr = 2; switch (addr) { @@ -412,7 +411,10 @@ static uint64_t xilinx_spips_read(void *opaque, hwaddr addr, mask = 0; break; case R_RX_DATA: -rx_data_bytes(s, ret, s-num_txrx_bytes); +memset(rx_buf, 0, sizeof(rx_buf)); +rx_data_bytes(s, rx_buf, s-num_txrx_bytes); +ret = s-regs[R_CONFIG] ENDIAN ? cpu_to_be32(*(uint32_t *)rx_buf) +: cpu_to_le32(*(uint32_t *)rx_buf); DB_PRINT_L(0, addr= TARGET_FMT_plx = %x\n, addr * 4, ret); xilinx_spips_update_ixr(s); return ret; @@ -525,7 +527,8 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size) if (addr = q-lqspi_cached_addr addr = q-lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) { -ret = q-lqspi_buf[(addr - q-lqspi_cached_addr) 2]; +uint8_t *retp = q-lqspi_buf[addr - q-lqspi_cached_addr]; +ret = cpu_to_le32(*(uint32_t *)retp); DB_PRINT_L(1, addr: %08x, data: %08x\n, (unsigned)addr, (unsigned)ret); return ret; @@ -571,13 +574,13 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size) DB_PRINT_L(0, starting QSPI data read\n); -while (cache_entry LQSPI_CACHE_SIZE / 4) { -for (i = 0; i 16; ++i) { -tx_data_bytes(s, 0, 4); +while (cache_entry LQSPI_CACHE_SIZE) { +for (i = 0; i 64; ++i) { +tx_data_bytes(s, 0, 1); } xilinx_spips_flush_txfifo(s); -for (i = 0; i 16; ++i) { -rx_data_bytes(s, q-lqspi_buf[cache_entry++], 4); +for (i = 0; i 64; ++i) { +rx_data_bytes(s, q-lqspi_buf[cache_entry++], 1); } } @@ -586,7 +589,7 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size) s-regs[R_CONFIG] = r_config_save; xilinx_spips_update_cs_lines(s); -q-lqspi_cached_addr = addr; +q-lqspi_cached_addr = flash_addr * num_effective_busses(s); return lqspi_read(opaque, addr, size); } } @@ -595,7 +598,7 @@ static const MemoryRegionOps lqspi_ops = { .read = lqspi_read, .endianness = DEVICE_NATIVE_ENDIAN, .valid = { -.min_access_size = 4, +.min_access_size = 1, .max_access_size = 4 } }; -- 1.7.0.4
[Qemu-devel] [PATCH arm-devs v1 0/1] cadence_uart: U-boot driver
Hi Peter, This is a corner case bug discovered by U-boot driver for cadence GEM. Regards, Peter Peter Crosthwaite (1): cadence_uart: Flush queued characters on reset hw/cadence_uart.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-)
[Qemu-devel] [PATCH arm-devs v1 1/1] cadence_uart: Flush queued characters on reset
Reset can be used to empty the rx-fifo. As the fifo full condition is used to return false from can_receive, queued rx data should be flushed on reset accordingly. Cc: Wendy Liang jli...@xilinx.com Cc: Jason Wu hua...@xilinx.com Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reported-by: Jason Wu hua...@xilinx.com --- hw/cadence_uart.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/cadence_uart.c b/hw/cadence_uart.c index 5426f10..421ec99 100644 --- a/hw/cadence_uart.c +++ b/hw/cadence_uart.c @@ -157,6 +157,7 @@ static void uart_rx_reset(UartState *s) { s-rx_wpos = 0; s-rx_count = 0; +qemu_chr_accept_input(s-chr); s-r[R_SR] |= UART_SR_INTR_REMPTY; s-r[R_SR] = ~UART_SR_INTR_RFUL; -- 1.7.0.4
Re: [Qemu-devel] [PATCH V3 WIP 3/3] disable vhost_verify_ring_mappings check
On Tue, 2013-04-02 at 21:04 -0700, Nicholas A. Bellinger wrote: On Tue, 2013-04-02 at 16:27 +0300, Michael S. Tsirkin wrote: On Mon, Apr 01, 2013 at 06:05:47PM -0700, Nicholas A. Bellinger wrote: On Fri, 2013-03-29 at 09:14 +0100, Paolo Bonzini wrote: Il 29/03/2013 03:53, Nicholas A. Bellinger ha scritto: On Thu, 2013-03-28 at 06:13 -0400, Paolo Bonzini wrote: I think it's the right thing to do, but maybe not the right place to do this, need to reset after all IO is done, before ring memory is write protected. Our emails are crossing each other unfortunately, but I want to reinforce this: ring memory is not write protected. Understood. However, AFAICT the act of write protecting these ranges for ROM generates the offending callbacks to vhost_set_memory(). The part that I'm missing is if ring memory is not being write protected by make_bios_readonly_intel(), why are the vhost_set_memory() calls being invoked..? Because mappings change for the region that contains the ring. vhost doesn't know yet that the changes do not affect ring memory, vhost_set_memory() is called exactly to ascertain that. SNIP Is it possible that what is going on here, is that we had a region at address 0x0 size 0x8000, and now a chunk from it is being made readonly, and to this end the whole old region is removed then new ones are added? Yes, I believe this is exactly what is happening.. If yes maybe the problem is that we don't use the atomic begin/commit ops in the memory API. Maybe the following will help? Completely untested, posting just to give you the idea: Mmmm, one question on how vhost_region_del() + vhost_region_add() + vhost_commit() should work.. Considering the following when the same seabios code snippet: pci_config_writeb(0x31): bdf: 0x pam: 0x005b is executed to mark an pc.ram area 0xc as readonly: Entering vhost_begin Entering vhost_region_del section: 0x7fd037a4bb60 offset_within_region: 0xc size: 2146697216 readonly: 0 vhost_region_del: is_rom: 0, rom_device: 0 vhost_region_del: readable: 1 vhost_region_del: ram_addr 0x0, addr: 0x0 size: 2147483648 vhost_region_del: name: pc.ram Entering vhost_set_memory, section: 0x7fd037a4bb60 add: 0, dev-started: 1 vhost_set_memory: Setting dev-memory_changed = true for start_addr: 0xc Entering vhost_region_add section: 0x7fd037a4baa0 offset_within_region: 0xc size: 32768 readonly: 1 vhost_region_add is readonly !!! vhost_region_add: is_rom: 0, rom_device: 0 vhost_region_add: readable: 1 vhost_region_add: ram_addr 0x, addr: 0x 0 size: 2147483648 vhost_region_add: name: pc.ram Entering vhost_set_memory, section: 0x7fd037a4baa0 add: 1, dev-started: 1 vhost_dev_assign_memory(); reg-guest_phys_addr: 0xc vhost_set_memory: Setting dev-memory_changed = true for start_addr: 0xc Entering vhost_region_add section: 0x7fd037a4baa0 offset_within_region: 0xc8000 size: 2146664448 readonly: 0 vhost_region_add: is_rom: 0, rom_device: 0 vhost_region_add: readable: 1 vhost_region_add: ram_addr 0x, addr: 0x 0 size: 2147483648 vhost_region_add: name: pc.ram Entering vhost_set_memory, section: 0x7fd037a4baa0 add: 1, dev-started: 1 vhost_set_memory: Setting dev-memory_changed = true for start_addr: 0xc8000 phys_page_find got PHYS_MAP_NODE_NIL .. Entering vhost_commit Note that originally we'd see the cpu_physical_memory_map() failure in vhost_verify_ring_mappings() after the first -region_del() above. Adding a hardcoded cpu_physical_memory_map() testcase in vhost_commit() for phys_addr=0xed000, len=5124 (vq ring) does locate the correct *section from address_space_map(), which correct points to the section generated by the last vhost_region_add() above: Entering vhost_commit address_space_map: addr: 0xed000, plen: 5124 address_space_map: l: 4096, len: 5124 address_space_map: section: 0x7f41b325f020 memory_region_is_ram: 1 readonly: 0 address_space_map: section: 0x7f41b325f020 offset_within_region: 0xc8000 section size: 2146664448 address_space_map: l: 4096, len: 1028 address_space_map: section: 0x7f41b325f020 memory_region_is_ram: 1 readonly: 0 address_space_map: section: 0x7f41b325f020 offset_within_region: 0xc8000 section size: 2146664448 address_space_map: Calling qemu_ram_ptr_length: raddr: 0x ed000 rlen: 5124 address_space_map: After qemu_ram_ptr_length: raddr: 0x ed000 rlen: 5124 cpu_physical_memory_map(0xed000) got l: 5124 So, does using a -commit callback for MemoryListener mean that vhost_verify_ring_mappings() is OK to be called only from the final -commit callback, and not from each -region_del + -region_add callback..? Eg: I seem to recall something about vhost_verify_ring_mappings() being called during each
Re: [Qemu-devel] Regarding migration
All of migration code can be found in migrate.c file. To understand migration, you can run qemu in gdb and place breakpoints in do_savevm() function on server side and do_loadvm() function on client side. Migration consists of the following parts: 1. save device driver state (11 in number. this consists of keyboard, mouse and other device drivers on server side) 2. RAM (this is also structured like other device drivers but there is a special way in which this device driver is saved). 3. Disk (saved in the form of block device). first you make a client to sleep over a port. i.e. client waits for server to initiate migration. on server, you give command to savevm. the entry point is do_savevm() from which 3 things happen: 1. qemu_savevm_state_begin() is called, ram is snapshotted for the first time, all pages are marked dirty and thus saved. 2. qemu_savevm_state_iterate() is called, from which ram is snapshotted time and again till either of the two things happen: 1. no of iterations go more than 30. 2. rate at which pages get dirty decreases in two successive iterations. [NOTE that till here only RAM was being saved, the rest of the 12 device drivers have not been saved yet] == vm_stop() is called. this leads to halt of server vm(); no more ram pages get dirty == 3. qemu_savevm_state_complete() is called where rest of the 12 device driver state is saved and 1 final iteration of saving ram is done. this is device, ram and block(disk) state transferred via tcp / file to client on a port / fd. (depending on what client was listening to) At client end, the reverse sequence is followed i,e. first you load the device driver, then the ram image and so on. and then vm_start() is called. Hence, entry point for starting migration = do_savevm(). entry point for receiving migration = do_loadvm(). more about live migration can be read in this amazing paper by Avi Kivity :: www.linux-*kvm*.com/sites/default/files/*kivity*-Reprint.pdf Regards, Shehbaz Jaffer Grad Student, IIT Delhi. On Wed, Apr 3, 2013 at 3:07 AM, pavan pavu.ku...@gmail.com wrote: Hi.. I was wondering what are the entry points for the migration at source and destination. My understanding is that at source the method do_migrate in migrate.c file is the entry point and at the destination the method main in vl.c is the entry point. Is this correct ? -- Pavan Kumar M Software Engineer.. not anymore though.. North Carolina State University -- Shehbaz Jaffer Graduate Student Department of Computer Engineering Indian Institute of Technology, Delhi
[Qemu-devel] [PATCH v5 00/16] Stream Patches
Hi all. The Xilinx AXIEnet and DMA devices have two AXI stream connections (control and data), only one of which is currently modelled (data). AXI stream is modelled using the stream QOM interface described in stream.h. Unfortunately, interfaces have no nice way of modelling multiple connections of the same type. So to overcome this I created a secondary object which acts as a proxy for the stream connection. Multiple connections can be implemented using multiple proxies and stream masters link to the relevant proxy, rather than the ethernet device itself. This Series changes AXI Enet and DMA to be connected as such. Also changed the stream interface to implement flow control handshaking. This is needed for the AXIEnet to be be able to implement the net can_receive() flow control. Patches 1-10 are low-impact cleanup of axienet/dma as per the current QOM styling guidelines and can be cherry-picked off the front. changed from v4: resynchronized control flow stream API reordered series for better consistency. Folded patch 17 in 16 (app array length fix) Fixed DMA tx path halted bit changed from v3: Changed from asynchronous flow control to synchronous (Edgar review) changed from v2: Reordered patches (from low impact - high impact) Added styling refactoring of AXIDMA Added asynchronous patches Removed dummy second stream connection patch (former patch 8) Added (functional) second stream connection changed from v1: Removed former P12 (already merged) Address Andreas review Refactor axienet to be more QOM friendly. Peter Crosthwaite (16): xilinx_axienet: typedef XilinxAXIEnet struct xilinx_axienet: Defined and use type cast macro xilinx_axienet: Register reset properly xilinx_axienet: converted init-realize xilinx_axidma: typedef XilinxAXIDMA struct xilinx_axidma: Defined and use type cast macro xilinx_axidma: Register reset properly xilinx_axidma: converted init-realize petalogix_ml605_mmu: Fix machine node attachment petalogix_ml605_mmu: Attach ethernet to machine xilinx_axienet: Create Proxy object for stream xilinx_axidma: Create Proxy object for stream xilinx_axidma: Fix rx/tx halted bit. stream: Add flow control API xilinx_axienet/dma: Implement rx path flow control stream: Remove app argument hack hw/microblaze/petalogix_ml605_mmu.c | 28 +++- hw/stream.c | 15 ++- hw/stream.h | 36 - hw/xilinx.h | 21 ++- hw/xilinx_axidma.c | 261 +++ hw/xilinx_axienet.c | 255 +++--- 6 files changed, 483 insertions(+), 133 deletions(-)
[Qemu-devel] [PATCH v5 01/16] xilinx_axienet: typedef XilinxAXIEnet struct
Typedef xilinx_axienets object state struct to shorten the repeated usages of struct XilinxAXIEnet. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Andreas Färber afaer...@suse.de Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- hw/xilinx_axienet.c | 44 +++- 1 files changed, 23 insertions(+), 21 deletions(-) diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c index 5785290..c3c0663 100644 --- a/hw/xilinx_axienet.c +++ b/hw/xilinx_axienet.c @@ -306,6 +306,8 @@ struct TEMAC { void *parent; }; +typedef struct XilinxAXIEnet XilinxAXIEnet; + struct XilinxAXIEnet { SysBusDevice busdev; MemoryRegion iomem; @@ -365,37 +367,37 @@ struct XilinxAXIEnet { uint8_t *rxmem; }; -static void axienet_rx_reset(struct XilinxAXIEnet *s) +static void axienet_rx_reset(XilinxAXIEnet *s) { s-rcw[1] = RCW1_JUM | RCW1_FCS | RCW1_RX | RCW1_VLAN; } -static void axienet_tx_reset(struct XilinxAXIEnet *s) +static void axienet_tx_reset(XilinxAXIEnet *s) { s-tc = TC_JUM | TC_TX | TC_VLAN; } -static inline int axienet_rx_resetting(struct XilinxAXIEnet *s) +static inline int axienet_rx_resetting(XilinxAXIEnet *s) { return s-rcw[1] RCW1_RST; } -static inline int axienet_rx_enabled(struct XilinxAXIEnet *s) +static inline int axienet_rx_enabled(XilinxAXIEnet *s) { return s-rcw[1] RCW1_RX; } -static inline int axienet_extmcf_enabled(struct XilinxAXIEnet *s) +static inline int axienet_extmcf_enabled(XilinxAXIEnet *s) { return !!(s-regs[R_RAF] RAF_EMCF_EN); } -static inline int axienet_newfunc_enabled(struct XilinxAXIEnet *s) +static inline int axienet_newfunc_enabled(XilinxAXIEnet *s) { return !!(s-regs[R_RAF] RAF_NEWFUNC_EN); } -static void axienet_reset(struct XilinxAXIEnet *s) +static void axienet_reset(XilinxAXIEnet *s) { axienet_rx_reset(s); axienet_tx_reset(s); @@ -406,7 +408,7 @@ static void axienet_reset(struct XilinxAXIEnet *s) s-emmc = EMMC_LINKSPEED_100MB; } -static void enet_update_irq(struct XilinxAXIEnet *s) +static void enet_update_irq(XilinxAXIEnet *s) { s-regs[R_IP] = s-regs[R_IS] s-regs[R_IE]; qemu_set_irq(s-irq, !!s-regs[R_IP]); @@ -414,7 +416,7 @@ static void enet_update_irq(struct XilinxAXIEnet *s) static uint64_t enet_read(void *opaque, hwaddr addr, unsigned size) { -struct XilinxAXIEnet *s = opaque; +XilinxAXIEnet *s = opaque; uint32_t r = 0; addr = 2; @@ -506,7 +508,7 @@ static uint64_t enet_read(void *opaque, hwaddr addr, unsigned size) static void enet_write(void *opaque, hwaddr addr, uint64_t value, unsigned size) { -struct XilinxAXIEnet *s = opaque; +XilinxAXIEnet *s = opaque; struct TEMAC *t = s-TEMAC; addr = 2; @@ -618,7 +620,7 @@ static const MemoryRegionOps enet_ops = { static int eth_can_rx(NetClientState *nc) { -struct XilinxAXIEnet *s = qemu_get_nic_opaque(nc); +XilinxAXIEnet *s = qemu_get_nic_opaque(nc); /* RX enabled? */ return !axienet_rx_resetting(s) axienet_rx_enabled(s); @@ -641,7 +643,7 @@ static int enet_match_addr(const uint8_t *buf, uint32_t f0, uint32_t f1) static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size) { -struct XilinxAXIEnet *s = qemu_get_nic_opaque(nc); +XilinxAXIEnet *s = qemu_get_nic_opaque(nc); static const unsigned char sa_bcast[6] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; static const unsigned char sa_ipmcast[3] = {0x01, 0x00, 0x52}; @@ -786,7 +788,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size) static void eth_cleanup(NetClientState *nc) { /* FIXME. */ -struct XilinxAXIEnet *s = qemu_get_nic_opaque(nc); +XilinxAXIEnet *s = qemu_get_nic_opaque(nc); g_free(s-rxmem); g_free(s); } @@ -794,7 +796,7 @@ static void eth_cleanup(NetClientState *nc) static void axienet_stream_push(StreamSlave *obj, uint8_t *buf, size_t size, uint32_t *hdr) { -struct XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj)); +XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj)); /* TX enable ? */ if (!(s-tc TC_TX)) { @@ -844,7 +846,7 @@ static NetClientInfo net_xilinx_enet_info = { static int xilinx_enet_init(SysBusDevice *dev) { -struct XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), dev); +XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), dev); sysbus_init_irq(dev, s-irq); @@ -869,7 +871,7 @@ static int xilinx_enet_init(SysBusDevice *dev) static void xilinx_enet_initfn(Object *obj) { -struct XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj)); +XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj)); Error *errp = NULL; object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE, @@ -878,10 +880,10 @@ static void xilinx_enet_initfn(Object *obj) } static Property
[Qemu-devel] [PATCH v5 02/16] xilinx_axienet: Defined and use type cast macro
Standard QOM cast macro. Replaces usages of FROM_SYSBUS Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Andreas Färber afaer...@suse.de Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- hw/xilinx_axienet.c | 11 --- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c index c3c0663..4c7d390 100644 --- a/hw/xilinx_axienet.c +++ b/hw/xilinx_axienet.c @@ -32,6 +32,11 @@ #define DPHY(x) +#define TYPE_XILINX_AXI_ENET xlnx.axi-ethernet + +#define XILINX_AXI_ENET(obj) \ + OBJECT_CHECK(XilinxAXIEnet, (obj), TYPE_XILINX_AXI_ENET) + /* Advertisement control register. */ #define ADVERTISE_10HALF0x0020 /* Try for 10mbps half-duplex */ #define ADVERTISE_10FULL0x0040 /* Try for 10mbps full-duplex */ @@ -846,7 +851,7 @@ static NetClientInfo net_xilinx_enet_info = { static int xilinx_enet_init(SysBusDevice *dev) { -XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), dev); +XilinxAXIEnet *s = XILINX_AXI_ENET(dev); sysbus_init_irq(dev, s-irq); @@ -871,7 +876,7 @@ static int xilinx_enet_init(SysBusDevice *dev) static void xilinx_enet_initfn(Object *obj) { -XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj)); +XilinxAXIEnet *s = XILINX_AXI_ENET(obj); Error *errp = NULL; object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE, @@ -899,7 +904,7 @@ static void xilinx_enet_class_init(ObjectClass *klass, void *data) } static const TypeInfo xilinx_enet_info = { -.name = xlnx.axi-ethernet, +.name = TYPE_XILINX_AXI_ENET, .parent= TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(XilinxAXIEnet), .class_init= xilinx_enet_class_init, -- 1.7.0.4
[Qemu-devel] [PATCH v5 03/16] xilinx_axienet: Register reset properly
Register the reset function and the Device::reset function rather than explicitly call it from the sysbus::init. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Andreas Färber afaer...@suse.de Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- hw/xilinx_axienet.c |6 -- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c index 4c7d390..35b4e4f 100644 --- a/hw/xilinx_axienet.c +++ b/hw/xilinx_axienet.c @@ -402,8 +402,10 @@ static inline int axienet_newfunc_enabled(XilinxAXIEnet *s) return !!(s-regs[R_RAF] RAF_NEWFUNC_EN); } -static void axienet_reset(XilinxAXIEnet *s) +static void xilinx_axienet_reset(DeviceState *d) { +XilinxAXIEnet *s = XILINX_AXI_ENET(d); + axienet_rx_reset(s); axienet_tx_reset(s); @@ -869,7 +871,6 @@ static int xilinx_enet_init(SysBusDevice *dev) s-TEMAC.parent = s; s-rxmem = g_malloc(s-c_rxmem); -axienet_reset(s); return 0; } @@ -900,6 +901,7 @@ static void xilinx_enet_class_init(ObjectClass *klass, void *data) k-init = xilinx_enet_init; dc-props = xilinx_enet_properties; +dc-reset = xilinx_axienet_reset; ssc-push = axienet_stream_push; } -- 1.7.0.4
[Qemu-devel] [PATCH v5 04/16] xilinx_axienet: converted init-realize
The prescribed transition from SysBusDevice::init to Device::realize. Im going with Andreas suggestion to move the sysbus foo to Object::init for early IRQ visibility. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Andreas Färber afaer...@suse.de Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- Changed from v3: Added missing include of qerror hw/xilinx_axidma.c |1 + hw/xilinx_axienet.c | 24 +++- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c index 8db1a74..5d2b33a 100644 --- a/hw/xilinx_axidma.c +++ b/hw/xilinx_axidma.c @@ -27,6 +27,7 @@ #include hw/ptimer.h #include qemu/log.h #include hw/qdev-addr.h +#include qapi/qmp/qerror.h #include hw/stream.h diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c index 35b4e4f..ec1e893 100644 --- a/hw/xilinx_axienet.c +++ b/hw/xilinx_axienet.c @@ -851,18 +851,13 @@ static NetClientInfo net_xilinx_enet_info = { .cleanup = eth_cleanup, }; -static int xilinx_enet_init(SysBusDevice *dev) +static void xilinx_enet_realize(DeviceState *dev, Error **errp) { XilinxAXIEnet *s = XILINX_AXI_ENET(dev); -sysbus_init_irq(dev, s-irq); - -memory_region_init_io(s-iomem, enet_ops, s, enet, 0x4); -sysbus_init_mmio(dev, s-iomem); - qemu_macaddr_default_if_unset(s-conf.macaddr); s-nic = qemu_new_nic(net_xilinx_enet_info, s-conf, - object_get_typename(OBJECT(dev)), dev-qdev.id, s); + object_get_typename(OBJECT(dev)), dev-id, s); qemu_format_nic_info_str(qemu_get_queue(s-nic), s-conf.macaddr.a); tdk_init(s-TEMAC.phy); @@ -871,18 +866,22 @@ static int xilinx_enet_init(SysBusDevice *dev) s-TEMAC.parent = s; s-rxmem = g_malloc(s-c_rxmem); - -return 0; } -static void xilinx_enet_initfn(Object *obj) +static void xilinx_enet_init(Object *obj) { XilinxAXIEnet *s = XILINX_AXI_ENET(obj); +SysBusDevice *sbd = SYS_BUS_DEVICE(obj); Error *errp = NULL; object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE, (Object **) s-tx_dev, errp); assert_no_error(errp); + +sysbus_init_irq(sbd, s-irq); + +memory_region_init_io(s-iomem, enet_ops, s, enet, 0x4); +sysbus_init_mmio(sbd, s-iomem); } static Property xilinx_enet_properties[] = { @@ -896,10 +895,9 @@ static Property xilinx_enet_properties[] = { static void xilinx_enet_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); StreamSlaveClass *ssc = STREAM_SLAVE_CLASS(klass); -k-init = xilinx_enet_init; +dc-realize = xilinx_enet_realize; dc-props = xilinx_enet_properties; dc-reset = xilinx_axienet_reset; ssc-push = axienet_stream_push; @@ -910,7 +908,7 @@ static const TypeInfo xilinx_enet_info = { .parent= TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(XilinxAXIEnet), .class_init= xilinx_enet_class_init, -.instance_init = xilinx_enet_initfn, +.instance_init = xilinx_enet_init, .interfaces = (InterfaceInfo[]) { { TYPE_STREAM_SLAVE }, { } -- 1.7.0.4
[Qemu-devel] [PATCH v5 05/16] xilinx_axidma: typedef XilinxAXIDMA struct
Typedef xilinx_axidma's object state struct to shorten the repeated usages of struct XilinxAXIDMA. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- hw/xilinx_axidma.c | 16 +--- 1 files changed, 9 insertions(+), 7 deletions(-) diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c index 5d2b33a..d57538e 100644 --- a/hw/xilinx_axidma.c +++ b/hw/xilinx_axidma.c @@ -39,6 +39,8 @@ #define R_TAILDESC (0x10 / 4) #define R_MAX (0x30 / 4) +typedef struct XilinxAXIDMA XilinxAXIDMA; + enum { DMACR_RUNSTOP = 1, DMACR_TAILPTR_MODE = 2, @@ -354,7 +356,7 @@ static void stream_process_s2mem(struct Stream *s, static void axidma_push(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app) { -struct XilinxAXIDMA *d = FROM_SYSBUS(typeof(*d), SYS_BUS_DEVICE(obj)); +XilinxAXIDMA *d = FROM_SYSBUS(typeof(*d), SYS_BUS_DEVICE(obj)); struct Stream *s = d-streams[1]; if (!app) { @@ -367,7 +369,7 @@ axidma_push(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app) static uint64_t axidma_read(void *opaque, hwaddr addr, unsigned size) { -struct XilinxAXIDMA *d = opaque; +XilinxAXIDMA *d = opaque; struct Stream *s; uint32_t r = 0; int sid; @@ -402,7 +404,7 @@ static uint64_t axidma_read(void *opaque, hwaddr addr, static void axidma_write(void *opaque, hwaddr addr, uint64_t value, unsigned size) { -struct XilinxAXIDMA *d = opaque; +XilinxAXIDMA *d = opaque; struct Stream *s; int sid; @@ -460,7 +462,7 @@ static const MemoryRegionOps axidma_ops = { static int xilinx_axidma_init(SysBusDevice *dev) { -struct XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), dev); +XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), dev); int i; sysbus_init_irq(dev, s-streams[0].irq); @@ -482,14 +484,14 @@ static int xilinx_axidma_init(SysBusDevice *dev) static void xilinx_axidma_initfn(Object *obj) { -struct XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj)); +XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj)); object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE, (Object **) s-tx_dev, NULL); } static Property axidma_properties[] = { -DEFINE_PROP_UINT32(freqhz, struct XilinxAXIDMA, freqhz, 5000), +DEFINE_PROP_UINT32(freqhz, XilinxAXIDMA, freqhz, 5000), DEFINE_PROP_END_OF_LIST(), }; @@ -507,7 +509,7 @@ static void axidma_class_init(ObjectClass *klass, void *data) static const TypeInfo axidma_info = { .name = xlnx.axi-dma, .parent= TYPE_SYS_BUS_DEVICE, -.instance_size = sizeof(struct XilinxAXIDMA), +.instance_size = sizeof(XilinxAXIDMA), .class_init= axidma_class_init, .instance_init = xilinx_axidma_initfn, .interfaces = (InterfaceInfo[]) { -- 1.7.0.4
[Qemu-devel] [PATCH v5 06/16] xilinx_axidma: Defined and use type cast macro
Standard QOM cast macro. Replaces usages of FROM_SYSBUS Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- hw/xilinx_axidma.c | 13 + 1 files changed, 9 insertions(+), 4 deletions(-) diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c index d57538e..7cb40bd 100644 --- a/hw/xilinx_axidma.c +++ b/hw/xilinx_axidma.c @@ -33,6 +33,11 @@ #define D(x) +#define TYPE_XILINX_AXI_DMA xlnx.axi-dma + +#define XILINX_AXI_DMA(obj) \ + OBJECT_CHECK(XilinxAXIDMA, (obj), TYPE_XILINX_AXI_DMA) + #define R_DMACR (0x00 / 4) #define R_DMASR (0x04 / 4) #define R_CURDESC (0x08 / 4) @@ -356,7 +361,7 @@ static void stream_process_s2mem(struct Stream *s, static void axidma_push(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app) { -XilinxAXIDMA *d = FROM_SYSBUS(typeof(*d), SYS_BUS_DEVICE(obj)); +XilinxAXIDMA *d = XILINX_AXI_DMA(obj); struct Stream *s = d-streams[1]; if (!app) { @@ -462,7 +467,7 @@ static const MemoryRegionOps axidma_ops = { static int xilinx_axidma_init(SysBusDevice *dev) { -XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), dev); +XilinxAXIDMA *s = XILINX_AXI_DMA(dev); int i; sysbus_init_irq(dev, s-streams[0].irq); @@ -484,7 +489,7 @@ static int xilinx_axidma_init(SysBusDevice *dev) static void xilinx_axidma_initfn(Object *obj) { -XilinxAXIDMA *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj)); +XilinxAXIDMA *s = XILINX_AXI_DMA(obj); object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE, (Object **) s-tx_dev, NULL); @@ -507,7 +512,7 @@ static void axidma_class_init(ObjectClass *klass, void *data) } static const TypeInfo axidma_info = { -.name = xlnx.axi-dma, +.name = TYPE_XILINX_AXI_DMA, .parent= TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(XilinxAXIDMA), .class_init= axidma_class_init, -- 1.7.0.4
[Qemu-devel] [PATCH v5 07/16] xilinx_axidma: Register reset properly
Register the reset function as the Device::reset function rather than explicitly call it from the sysbus::init. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- hw/xilinx_axidma.c | 12 +++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c index 7cb40bd..ac62245 100644 --- a/hw/xilinx_axidma.c +++ b/hw/xilinx_axidma.c @@ -358,6 +358,16 @@ static void stream_process_s2mem(struct Stream *s, } } +static void xilinx_axidma_reset(DeviceState *dev) +{ +int i; +XilinxAXIDMA *s = XILINX_AXI_DMA(dev); + +for (i = 0; i 2; i++) { +stream_reset(s-streams[i]); +} +} + static void axidma_push(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app) { @@ -478,7 +488,6 @@ static int xilinx_axidma_init(SysBusDevice *dev) sysbus_init_mmio(dev, s-iomem); for (i = 0; i 2; i++) { -stream_reset(s-streams[i]); s-streams[i].nr = i; s-streams[i].bh = qemu_bh_new(timer_hit, s-streams[i]); s-streams[i].ptimer = ptimer_init(s-streams[i].bh); @@ -507,6 +516,7 @@ static void axidma_class_init(ObjectClass *klass, void *data) StreamSlaveClass *ssc = STREAM_SLAVE_CLASS(klass); k-init = xilinx_axidma_init; +dc-reset = xilinx_axidma_reset; dc-props = axidma_properties; ssc-push = axidma_push; } -- 1.7.0.4
[Qemu-devel] [PATCH v5 08/16] xilinx_axidma: converted init-realize
The prescribed transition from SysBusDevice::init to Device::realize. I'm going with Andreas suggestion to move the sysbus foo to Object::init for early IRQ visibility. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- hw/xilinx_axidma.c | 25 - 1 files changed, 12 insertions(+), 13 deletions(-) diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c index ac62245..2c95765 100644 --- a/hw/xilinx_axidma.c +++ b/hw/xilinx_axidma.c @@ -475,33 +475,33 @@ static const MemoryRegionOps axidma_ops = { .endianness = DEVICE_NATIVE_ENDIAN, }; -static int xilinx_axidma_init(SysBusDevice *dev) +static void xilinx_axidma_realize(DeviceState *dev, Error **errp) { XilinxAXIDMA *s = XILINX_AXI_DMA(dev); int i; -sysbus_init_irq(dev, s-streams[0].irq); -sysbus_init_irq(dev, s-streams[1].irq); - -memory_region_init_io(s-iomem, axidma_ops, s, - xlnx.axi-dma, R_MAX * 4 * 2); -sysbus_init_mmio(dev, s-iomem); - for (i = 0; i 2; i++) { s-streams[i].nr = i; s-streams[i].bh = qemu_bh_new(timer_hit, s-streams[i]); s-streams[i].ptimer = ptimer_init(s-streams[i].bh); ptimer_set_freq(s-streams[i].ptimer, s-freqhz); } -return 0; } -static void xilinx_axidma_initfn(Object *obj) +static void xilinx_axidma_init(Object *obj) { XilinxAXIDMA *s = XILINX_AXI_DMA(obj); +SysBusDevice *sbd = SYS_BUS_DEVICE(obj); object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE, (Object **) s-tx_dev, NULL); + +sysbus_init_irq(sbd, s-streams[0].irq); +sysbus_init_irq(sbd, s-streams[1].irq); + +memory_region_init_io(s-iomem, axidma_ops, s, + xlnx.axi-dma, R_MAX * 4 * 2); +sysbus_init_mmio(sbd, s-iomem); } static Property axidma_properties[] = { @@ -512,10 +512,9 @@ static Property axidma_properties[] = { static void axidma_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass); StreamSlaveClass *ssc = STREAM_SLAVE_CLASS(klass); -k-init = xilinx_axidma_init; +dc-realize = xilinx_axidma_realize, dc-reset = xilinx_axidma_reset; dc-props = axidma_properties; ssc-push = axidma_push; @@ -526,7 +525,7 @@ static const TypeInfo axidma_info = { .parent= TYPE_SYS_BUS_DEVICE, .instance_size = sizeof(XilinxAXIDMA), .class_init= axidma_class_init, -.instance_init = xilinx_axidma_initfn, +.instance_init = xilinx_axidma_init, .interfaces = (InterfaceInfo[]) { { TYPE_STREAM_SLAVE }, { } -- 1.7.0.4
[Qemu-devel] [PATCH v5 09/16] petalogix_ml605_mmu: Fix machine node attachment
Just attach devices straight to the root machine node, rather than the unattached node Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Andreas Färber afaer...@suse.de Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- Suggested (indirectly) by Andreas if he wants to put his Suggested-by to it. hw/microblaze/petalogix_ml605_mmu.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c index cfc0220..ca918b9 100644 --- a/hw/microblaze/petalogix_ml605_mmu.c +++ b/hw/microblaze/petalogix_ml605_mmu.c @@ -134,8 +134,8 @@ petalogix_ml605_init(QEMUMachineInitArgs *args) dma = qdev_create(NULL, xlnx.axi-dma); /* FIXME: attach to the sysbus instead */ -object_property_add_child(container_get(qdev_get_machine(), /unattached), - xilinx-dma, OBJECT(dma), NULL); +object_property_add_child(qdev_get_machine(), xilinx-dma, OBJECT(dma), + NULL); xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(dma), 0x8278, irq[3], 0x1000, 0x1000); -- 1.7.0.4
[Qemu-devel] [PATCH v5 10/16] petalogix_ml605_mmu: Attach ethernet to machine
Explicitly make the ethernet a child of the machine. This is needed to set and use links pre-realize. Also makes the ethernet initialization consistent with its peer DMA. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Reviewed-by: Andreas Färber afaer...@suse.de Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- hw/microblaze/petalogix_ml605_mmu.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c index ca918b9..482c21f 100644 --- a/hw/microblaze/petalogix_ml605_mmu.c +++ b/hw/microblaze/petalogix_ml605_mmu.c @@ -134,6 +134,8 @@ petalogix_ml605_init(QEMUMachineInitArgs *args) dma = qdev_create(NULL, xlnx.axi-dma); /* FIXME: attach to the sysbus instead */ +object_property_add_child(qdev_get_machine(), xilinx-eth, OBJECT(eth0), + NULL); object_property_add_child(qdev_get_machine(), xilinx-dma, OBJECT(dma), NULL); -- 1.7.0.4
[Qemu-devel] [PATCH v5 11/16] xilinx_axienet: Create Proxy object for stream
Create a separate child object to proxy the stream slave connection. This is setup for future work where a second stream slave connection is needed. The new child object is created at qdev init time and is linked back to the parent (the ethernet device itself) automatically. Stream slave masters differentiate which slave connection they are connected to by linking to the proxy object rather than the parent. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- changed from v3: Fixed function name s/axienet_data_stream_push/xilinx_axienet_data_stream_push changed from v2: got rid or overly defensive assert change from v1: renamed data-stream proxy link to axistream-connected-target rebased ontop of realize conversion reworked error return mechanism (Andreas review) inlined child device state struct into parents (Andreas review) replaced object_new() - object_initialize() (Andreas review) hw/microblaze/petalogix_ml605_mmu.c |5 ++- hw/xilinx_axienet.c | 60 -- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c index 482c21f..9240660 100644 --- a/hw/microblaze/petalogix_ml605_mmu.c +++ b/hw/microblaze/petalogix_ml605_mmu.c @@ -79,6 +79,7 @@ petalogix_ml605_init(QEMUMachineInitArgs *args) const char *cpu_model = args-cpu_model; MemoryRegion *address_space_mem = get_system_memory(); DeviceState *dev, *dma, *eth0; +Object *peer; MicroBlazeCPU *cpu; SysBusDevice *busdev; CPUMBState *env; @@ -142,7 +143,9 @@ petalogix_ml605_init(QEMUMachineInitArgs *args) xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(dma), 0x8278, irq[3], 0x1000, 0x1000); -xilinx_axidma_init(dma, STREAM_SLAVE(eth0), 0x8460, irq[1], irq[0], +peer = object_property_get_link(OBJECT(eth0), +axistream-connected-target, NULL); +xilinx_axidma_init(dma, STREAM_SLAVE(peer), 0x8460, irq[1], irq[0], 100 * 100); { diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c index ec1e893..141d2ac 100644 --- a/hw/xilinx_axienet.c +++ b/hw/xilinx_axienet.c @@ -33,10 +33,15 @@ #define DPHY(x) #define TYPE_XILINX_AXI_ENET xlnx.axi-ethernet +#define TYPE_XILINX_AXI_ENET_DATA_STREAM xilinx-axienet-data-stream #define XILINX_AXI_ENET(obj) \ OBJECT_CHECK(XilinxAXIEnet, (obj), TYPE_XILINX_AXI_ENET) +#define XILINX_AXI_ENET_DATA_STREAM(obj) \ + OBJECT_CHECK(XilinxAXIEnetStreamSlave, (obj),\ + TYPE_XILINX_AXI_ENET_DATA_STREAM) + /* Advertisement control register. */ #define ADVERTISE_10HALF0x0020 /* Try for 10mbps half-duplex */ #define ADVERTISE_10FULL0x0040 /* Try for 10mbps full-duplex */ @@ -311,13 +316,21 @@ struct TEMAC { void *parent; }; +typedef struct XilinxAXIEnetStreamSlave XilinxAXIEnetStreamSlave; typedef struct XilinxAXIEnet XilinxAXIEnet; +struct XilinxAXIEnetStreamSlave { +Object parent; + +struct XilinxAXIEnet *enet; +} ; + struct XilinxAXIEnet { SysBusDevice busdev; MemoryRegion iomem; qemu_irq irq; StreamSlave *tx_dev; +XilinxAXIEnetStreamSlave rx_data_dev; NICState *nic; NICConf conf; @@ -801,9 +814,11 @@ static void eth_cleanup(NetClientState *nc) } static void -axienet_stream_push(StreamSlave *obj, uint8_t *buf, size_t size, uint32_t *hdr) +xilinx_axienet_data_stream_push(StreamSlave *obj, uint8_t *buf, size_t size, +uint32_t *hdr) { -XilinxAXIEnet *s = FROM_SYSBUS(typeof(*s), SYS_BUS_DEVICE(obj)); +XilinxAXIEnetStreamSlave *ds = XILINX_AXI_ENET_DATA_STREAM(obj); +XilinxAXIEnet *s = ds-enet; /* TX enable ? */ if (!(s-tc TC_TX)) { @@ -854,6 +869,18 @@ static NetClientInfo net_xilinx_enet_info = { static void xilinx_enet_realize(DeviceState *dev, Error **errp) { XilinxAXIEnet *s = XILINX_AXI_ENET(dev); +XilinxAXIEnetStreamSlave *ds = XILINX_AXI_ENET_DATA_STREAM(s-rx_data_dev); +Error *local_errp = NULL; + +object_property_add_link(OBJECT(ds), enet, xlnx.axi-ethernet, + (Object **) ds-enet, local_errp); +if (local_errp) { +goto xilinx_enet_realize_fail; +} +object_property_set_link(OBJECT(ds), OBJECT(s), enet, local_errp); +if (local_errp) { +goto xilinx_enet_realize_fail; +} qemu_macaddr_default_if_unset(s-conf.macaddr); s-nic = qemu_new_nic(net_xilinx_enet_info, s-conf, @@ -866,6 +893,12 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp) s-TEMAC.parent = s; s-rxmem = g_malloc(s-c_rxmem); +return; + +xilinx_enet_realize_fail: +if (!*errp) { +*errp = local_errp; +} } static void xilinx_enet_init(Object *obj) @@ -878,6 +911,11 @@ static void xilinx_enet_init(Object *obj) (Object **)
Re: [Qemu-devel] [PATCH V11 05/17] block: add snapshot info query function bdrv_query_snapshot_info_list()
于 2013-4-3 9:17, Eric Blake 写道: On 04/02/2013 05:47 AM, Wenchao Xia wrote: This patch adds function bdrv_query_snapshot_info_list(), which will retrieve snapshot info of an image in qmp object format. The implementation is based on the code moved from qemu-img.c with modification to fit more for qmp based block layer API. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- block/qapi.c | 55 ++--- include/block/qapi.h |4 ++- qemu-img.c |5 +++- 3 files changed, 49 insertions(+), 15 deletions(-) +/* + * return 0 on success, @p_list will be set only on success, and caller need to s/need/needs/ + * check *p_list on success. I wonder if this wording would be any better: Returns 0 on success, with *p_list either set to describe snapshot information, or NULL because there are no snapshots. Returns -1 on error, with *p_list untouched. + */ +int bdrv_query_snapshot_info_list(BlockDriverState *bs, + SnapshotInfoList **p_list, + Error **errp) { At any rate, my only commentary was on grammar and a possible wording for a comment, while the code itself is fine from my viewpoint; so feel free to add: Reviewed-by: Eric Blake ebl...@redhat.com +++ b/qemu-img.c @@ -1735,7 +1735,10 @@ static ImageInfoList *collect_image_info_list(const char *filename, info = g_new0(ImageInfo, 1); bdrv_collect_image_info(bs, info, filename); -bdrv_collect_snapshots(bs, info); +if (!bdrv_query_snapshot_info_list(bs, info-snapshots, NULL) +info-snapshots) { +info-has_snapshots = true; +} Hmm. info-snapshots starts life as NULL (thanks to g_new0), and is untouched on error. Since you are ignoring any errors, you technically could write: bdrv_query_snapshot_info_list(bs, info-snapshots, NULL); if (info-snapshots) { info-has_snapshots = true; } for the same semantics. That means that as of this commit, no caller cares about the return value of bdrv_query_snapshot_info_list (they only care about whether info-snapshots was changed to non-null), so it could return void for a slightly simpler implementation. But I don't know if any later patches in the series start to care about which error was returned. Yes it would be cared in ImageInfo retrieving later. This section in qemu-img.c would be replaced later so did not check strictly the returned value. -- Best Regards Wenchao Xia
[Qemu-devel] [PATCH v5 12/16] xilinx_axidma: Create Proxy object for stream
Create a separate child object to proxy the stream slave connection. This is setup for future work where a second stream slave connection is needed. The new child object is created at qdev init time and is linked back to the parent (the ethernet device itself) automatically. Stream slave masters differentiate which slave connection they are connected to by linking to the proxy object rather than the parent. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- changed since v3: Rebased to occur before flow control patches hw/microblaze/petalogix_ml605_mmu.c |6 ++- hw/xilinx_axidma.c | 63 --- 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c index 9240660..7581275 100644 --- a/hw/microblaze/petalogix_ml605_mmu.c +++ b/hw/microblaze/petalogix_ml605_mmu.c @@ -140,8 +140,10 @@ petalogix_ml605_init(QEMUMachineInitArgs *args) object_property_add_child(qdev_get_machine(), xilinx-dma, OBJECT(dma), NULL); -xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(dma), - 0x8278, irq[3], 0x1000, 0x1000); +peer = object_property_get_link(OBJECT(dma), +axistream-connected-target, NULL); +xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(peer), +0x8278, irq[3], 0x1000, 0x1000); peer = object_property_get_link(OBJECT(eth0), axistream-connected-target, NULL); diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c index 2c95765..02700ea 100644 --- a/hw/xilinx_axidma.c +++ b/hw/xilinx_axidma.c @@ -34,10 +34,15 @@ #define D(x) #define TYPE_XILINX_AXI_DMA xlnx.axi-dma +#define TYPE_XILINX_AXI_DMA_DATA_STREAM xilinx-axi-dma-data-stream #define XILINX_AXI_DMA(obj) \ OBJECT_CHECK(XilinxAXIDMA, (obj), TYPE_XILINX_AXI_DMA) +#define XILINX_AXI_DMA_DATA_STREAM(obj) \ + OBJECT_CHECK(XilinxAXIDMAStreamSlave, (obj),\ + TYPE_XILINX_AXI_DMA_DATA_STREAM) + #define R_DMACR (0x00 / 4) #define R_DMASR (0x04 / 4) #define R_CURDESC (0x08 / 4) @@ -45,6 +50,7 @@ #define R_MAX (0x30 / 4) typedef struct XilinxAXIDMA XilinxAXIDMA; +typedef struct XilinxAXIDMAStreamSlave XilinxAXIDMAStreamSlave; enum { DMACR_RUNSTOP = 1, @@ -97,11 +103,18 @@ struct Stream { uint32_t regs[R_MAX]; }; +struct XilinxAXIDMAStreamSlave { +Object parent; + +struct XilinxAXIDMA *dma; +}; + struct XilinxAXIDMA { SysBusDevice busdev; MemoryRegion iomem; uint32_t freqhz; StreamSlave *tx_dev; +XilinxAXIDMAStreamSlave rx_data_dev; struct Stream streams[2]; }; @@ -369,10 +382,11 @@ static void xilinx_axidma_reset(DeviceState *dev) } static void -axidma_push(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app) +xilinx_axidma_data_stream_push(StreamSlave *obj, unsigned char *buf, size_t len, + uint32_t *app) { -XilinxAXIDMA *d = XILINX_AXI_DMA(obj); -struct Stream *s = d-streams[1]; +XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(obj); +struct Stream *s = ds-dma-streams[1]; if (!app) { hw_error(No stream app data!\n); @@ -478,6 +492,19 @@ static const MemoryRegionOps axidma_ops = { static void xilinx_axidma_realize(DeviceState *dev, Error **errp) { XilinxAXIDMA *s = XILINX_AXI_DMA(dev); +XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(s-rx_data_dev); +Error *local_errp = NULL; + +object_property_add_link(OBJECT(ds), dma, TYPE_XILINX_AXI_DMA, + (Object **)ds-dma, local_errp); +if (local_errp) { +goto xilinx_axidma_realize_fail; +} +object_property_set_link(OBJECT(ds), OBJECT(s), dma, local_errp); +if (local_errp) { +goto xilinx_axidma_realize_fail; +} + int i; for (i = 0; i 2; i++) { @@ -486,16 +513,28 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp) s-streams[i].ptimer = ptimer_init(s-streams[i].bh); ptimer_set_freq(s-streams[i].ptimer, s-freqhz); } +return; + +xilinx_axidma_realize_fail: +if (!*errp) { +*errp = local_errp; +} } static void xilinx_axidma_init(Object *obj) { XilinxAXIDMA *s = XILINX_AXI_DMA(obj); SysBusDevice *sbd = SYS_BUS_DEVICE(obj); +Error *errp = NULL; object_property_add_link(obj, axistream-connected, TYPE_STREAM_SLAVE, (Object **) s-tx_dev, NULL); +object_initialize(s-rx_data_dev, TYPE_XILINX_AXI_DMA_DATA_STREAM); +object_property_add_child(OBJECT(s), axistream-connected-target, + (Object *)s-rx_data_dev, errp); +assert_no_error(errp); + sysbus_init_irq(sbd, s-streams[0].irq); sysbus_init_irq(sbd,
[Qemu-devel] [PATCH v5 14/16] stream: Add flow control API
Add basic flow control to stream. A stream slave may return short, indicating that it is not capable of accepting any more data at the present time. Polling or a callback can be used via the can_push() function to determine when the slave can receive again. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/stream.c | 13 +++-- hw/stream.h | 33 ++--- hw/xilinx_axidma.c |3 ++- hw/xilinx_axienet.c |8 +--- 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/hw/stream.c b/hw/stream.c index a07d6a5..5397a8d 100644 --- a/hw/stream.c +++ b/hw/stream.c @@ -1,11 +1,20 @@ #include hw/stream.h -void +size_t stream_push(StreamSlave *sink, uint8_t *buf, size_t len, uint32_t *app) { StreamSlaveClass *k = STREAM_SLAVE_GET_CLASS(sink); -k-push(sink, buf, len, app); +return k-push(sink, buf, len, app); +} + +bool +stream_can_push(StreamSlave *sink, StreamCanPushNotifyFn notify, +void *notify_opaque) +{ +StreamSlaveClass *k = STREAM_SLAVE_GET_CLASS(sink); + +return k-can_push ? k-can_push(sink, notify, notify_opaque) : true; } static const TypeInfo stream_slave_info = { diff --git a/hw/stream.h b/hw/stream.h index f6137d6..ff2cb14 100644 --- a/hw/stream.h +++ b/hw/stream.h @@ -18,14 +18,41 @@ typedef struct StreamSlave { Object Parent; } StreamSlave; +typedef void (*StreamCanPushNotifyFn)(void *opaque); + typedef struct StreamSlaveClass { InterfaceClass parent; - -void (*push)(StreamSlave *obj, unsigned char *buf, size_t len, +/** + * can push - determine if a stream slave is capable of accepting at least + * one byte of data. Returns false if cannot accept. If not implemented, the + * slave is assumed to always be capable of recieveing. + * @notify: Optional callback that the slave will call when the slave is + * capable of recieving again. Only called if false is returned. + * @notify_opaque: opaque data to pass to notify call. + */ +bool (*can_push)(StreamSlave *obj, StreamCanPushNotifyFn notify, + void *notify_opaque); +/** + * push - push data to a Stream slave. The number of bytes pushed is + * returned. If the slave short returns, the master must wait before trying + * again, the slave may continue to just return 0 waiting for the vm time to + * advance. The can_push() function can be used to trap the point in time + * where the slave is ready to recieve agai, otherwise polling on a QEMU + * timer will work. + * @obj: Stream slave to push to + * @buf: Data to write + * @len: Maximum number of bytes to write + */ +size_t (*push)(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app); } StreamSlaveClass; -void +size_t stream_push(StreamSlave *sink, uint8_t *buf, size_t len, uint32_t *app); +bool +stream_can_push(StreamSlave *sink, StreamCanPushNotifyFn notify, +void *notify_opaque); + + #endif /* STREAM_H */ diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c index 2bbfea1..80ce57f 100644 --- a/hw/xilinx_axidma.c +++ b/hw/xilinx_axidma.c @@ -381,7 +381,7 @@ static void xilinx_axidma_reset(DeviceState *dev) } } -static void +static size_t xilinx_axidma_data_stream_push(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app) { @@ -393,6 +393,7 @@ xilinx_axidma_data_stream_push(StreamSlave *obj, unsigned char *buf, size_t len, } stream_process_s2mem(s, buf, len, app); stream_update_irq(s); +return len; } static uint64_t axidma_read(void *opaque, hwaddr addr, diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c index 141d2ac..5c0ea59 100644 --- a/hw/xilinx_axienet.c +++ b/hw/xilinx_axienet.c @@ -813,7 +813,7 @@ static void eth_cleanup(NetClientState *nc) g_free(s); } -static void +static size_t xilinx_axienet_data_stream_push(StreamSlave *obj, uint8_t *buf, size_t size, uint32_t *hdr) { @@ -822,13 +822,13 @@ xilinx_axienet_data_stream_push(StreamSlave *obj, uint8_t *buf, size_t size, /* TX enable ? */ if (!(s-tc TC_TX)) { -return; +return size; } /* Jumbo or vlan sizes ? */ if (!(s-tc TC_JUM)) { if (size 1518 size = 1522 !(s-tc TC_VLAN)) { -return; +return size; } } @@ -856,6 +856,8 @@ xilinx_axienet_data_stream_push(StreamSlave *obj, uint8_t *buf, size_t size, s-stats.tx_bytes += size; s-regs[R_IS] |= IS_TX_COMPLETE; enet_update_irq(s); + +return size; } static NetClientInfo net_xilinx_enet_info = { -- 1.7.0.4
[Qemu-devel] [PATCH v5 13/16] xilinx_axidma: Fix rx/tx halted bit.
If there is no DMA buffer descriptor, the DMA halts, not idles. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com Acked-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- changed from v3: Fixed for TX path as well as RX hw/xilinx_axidma.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c index 02700ea..2bbfea1 100644 --- a/hw/xilinx_axidma.c +++ b/hw/xilinx_axidma.c @@ -276,7 +276,7 @@ static void stream_process_mem2s(struct Stream *s, stream_desc_load(s, s-regs[R_CURDESC]); if (s-desc.status SDESC_STATUS_COMPLETE) { -s-regs[R_DMASR] |= DMASR_IDLE; +s-regs[R_DMASR] |= DMASR_HALTED; break; } @@ -331,7 +331,7 @@ static void stream_process_s2mem(struct Stream *s, stream_desc_load(s, s-regs[R_CURDESC]); if (s-desc.status SDESC_STATUS_COMPLETE) { -s-regs[R_DMASR] |= DMASR_IDLE; +s-regs[R_DMASR] |= DMASR_HALTED; break; } -- 1.7.0.4
[Qemu-devel] [PATCH v5 15/16] xilinx_axienet/dma: Implement rx path flow control
Implement flow control for the RX data path from xilinx_axienet-xilinx_axidma. On short return from axidma, then ethernet sets up the notify callback to resume transfer from where it left off. This also allows the ethernet to track whether there is an in progress transaction and return false from ethernet can_receive() as appropriate. If the DMA backs up or is disabled it waits for enablement. When the rx stream IO region is touched, the can_push() notify function is called if set. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/xilinx_axidma.c | 49 + hw/xilinx_axienet.c | 28 +--- 2 files changed, 66 insertions(+), 11 deletions(-) diff --git a/hw/xilinx_axidma.c b/hw/xilinx_axidma.c index 80ce57f..a5bf102 100644 --- a/hw/xilinx_axidma.c +++ b/hw/xilinx_axidma.c @@ -117,6 +117,9 @@ struct XilinxAXIDMA { XilinxAXIDMAStreamSlave rx_data_dev; struct Stream streams[2]; + +StreamCanPushNotifyFn notify; +void *notify_opaque; }; /* @@ -315,16 +318,16 @@ static void stream_process_mem2s(struct Stream *s, } } -static void stream_process_s2mem(struct Stream *s, - unsigned char *buf, size_t len, uint32_t *app) +static size_t stream_process_s2mem(struct Stream *s, unsigned char *buf, + size_t len, uint32_t *app) { uint32_t prev_d; unsigned int rxlen; -int pos = 0; +size_t pos = 0; int sof = 1; if (!stream_running(s) || stream_idle(s)) { -return; +return 0; } while (len) { @@ -369,6 +372,8 @@ static void stream_process_s2mem(struct Stream *s, break; } } + +return pos; } static void xilinx_axidma_reset(DeviceState *dev) @@ -381,19 +386,37 @@ static void xilinx_axidma_reset(DeviceState *dev) } } +static bool +xilinx_axidma_data_stream_can_push(StreamSlave *obj, + StreamCanPushNotifyFn notify, + void *notify_opaque) +{ +XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(obj); +struct Stream *s = ds-dma-streams[1]; + +if (!stream_running(s) || stream_idle(s)) { +ds-dma-notify = notify; +ds-dma-notify_opaque = notify_opaque; +return false; +} + +return true; +} + static size_t xilinx_axidma_data_stream_push(StreamSlave *obj, unsigned char *buf, size_t len, uint32_t *app) { XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(obj); struct Stream *s = ds-dma-streams[1]; +size_t ret; if (!app) { hw_error(No stream app data!\n); } -stream_process_s2mem(s, buf, len, app); +ret = stream_process_s2mem(s, buf, len, app); stream_update_irq(s); -return len; +return ret; } static uint64_t axidma_read(void *opaque, hwaddr addr, @@ -481,6 +504,10 @@ static void axidma_write(void *opaque, hwaddr addr, s-regs[addr] = value; break; } +if (sid == 1 d-notify) { +d-notify(d-notify_opaque); +d-notify = NULL; +} stream_update_irq(s); } @@ -558,11 +585,17 @@ static void axidma_class_init(ObjectClass *klass, void *data) dc-props = axidma_properties; } +static StreamSlaveClass xilinx_axidma_data_stream_class = { +.push = xilinx_axidma_data_stream_push, +.can_push = xilinx_axidma_data_stream_can_push, +}; + static void xilinx_axidma_stream_class_init(ObjectClass *klass, void *data) { StreamSlaveClass *ssc = STREAM_SLAVE_CLASS(klass); -ssc-push = data; +ssc-push = ((StreamSlaveClass *)data)-push; +ssc-can_push = ((StreamSlaveClass *)data)-can_push; } static const TypeInfo axidma_info = { @@ -578,7 +611,7 @@ static const TypeInfo xilinx_axidma_data_stream_info = { .parent= TYPE_OBJECT, .instance_size = sizeof(struct XilinxAXIDMAStreamSlave), .class_init= xilinx_axidma_stream_class_init, -.class_data= xilinx_axidma_data_stream_push, +.class_data= xilinx_axidma_data_stream_class, .interfaces = (InterfaceInfo[]) { { TYPE_STREAM_SLAVE }, { } diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c index 5c0ea59..e67a68b 100644 --- a/hw/xilinx_axienet.c +++ b/hw/xilinx_axienet.c @@ -383,6 +383,9 @@ struct XilinxAXIEnet { uint8_t *rxmem; +uint32_t *rxapp; +uint32_t rxsize; +uint32_t rxpos; }; static void axienet_rx_reset(XilinxAXIEnet *s) @@ -643,7 +646,7 @@ static int eth_can_rx(NetClientState *nc) XilinxAXIEnet *s = qemu_get_nic_opaque(nc); /* RX enabled? */ -return !axienet_rx_resetting(s) axienet_rx_enabled(s); +return !s-rxsize !axienet_rx_resetting(s) axienet_rx_enabled(s); } static int enet_match_addr(const uint8_t *buf, uint32_t f0, uint32_t f1) @@ -661,6 +664,23 @@ static int enet_match_addr(const uint8_t *buf,
[Qemu-devel] [PATCH v5 16/16] stream: Remove app argument hack
The uint32_t *app argument doesn't exist in real hardware. It was a hack in xilinx_axidma/enet to fake the (secondary) control stream connection. Removed the argument and added the second stream to axienet/dma. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- changed since v3: Rebased against now synchronous stream control API Macrofied stream control packet lengths hw/microblaze/petalogix_ml605_mmu.c | 25 + hw/stream.c |4 +- hw/stream.h |5 +- hw/xilinx.h | 21 +--- hw/xilinx_axidma.c | 99 -- hw/xilinx_axienet.c | 100 +- 6 files changed, 187 insertions(+), 67 deletions(-) diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c index 7581275..341cd2e 100644 --- a/hw/microblaze/petalogix_ml605_mmu.c +++ b/hw/microblaze/petalogix_ml605_mmu.c @@ -79,7 +79,7 @@ petalogix_ml605_init(QEMUMachineInitArgs *args) const char *cpu_model = args-cpu_model; MemoryRegion *address_space_mem = get_system_memory(); DeviceState *dev, *dma, *eth0; -Object *peer; +Object *ds, *cs; MicroBlazeCPU *cpu; SysBusDevice *busdev; CPUMBState *env; @@ -140,15 +140,20 @@ petalogix_ml605_init(QEMUMachineInitArgs *args) object_property_add_child(qdev_get_machine(), xilinx-dma, OBJECT(dma), NULL); -peer = object_property_get_link(OBJECT(dma), -axistream-connected-target, NULL); -xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(peer), -0x8278, irq[3], 0x1000, 0x1000); - -peer = object_property_get_link(OBJECT(eth0), -axistream-connected-target, NULL); -xilinx_axidma_init(dma, STREAM_SLAVE(peer), 0x8460, irq[1], irq[0], - 100 * 100); +ds = object_property_get_link(OBJECT(dma), + axistream-connected-target, NULL); +cs = object_property_get_link(OBJECT(dma), + axistream-control-connected-target, NULL); +xilinx_axiethernet_init(eth0, nd_table[0], STREAM_SLAVE(ds), +STREAM_SLAVE(cs), 0x8278, irq[3], 0x1000, +0x1000); + +ds = object_property_get_link(OBJECT(eth0), + axistream-connected-target, NULL); +cs = object_property_get_link(OBJECT(eth0), + axistream-control-connected-target, NULL); +xilinx_axidma_init(dma, STREAM_SLAVE(ds), STREAM_SLAVE(cs), 0x8460, + irq[1], irq[0], 100 * 100); { SSIBus *spi; diff --git a/hw/stream.c b/hw/stream.c index 5397a8d..e6a05a5 100644 --- a/hw/stream.c +++ b/hw/stream.c @@ -1,11 +1,11 @@ #include hw/stream.h size_t -stream_push(StreamSlave *sink, uint8_t *buf, size_t len, uint32_t *app) +stream_push(StreamSlave *sink, uint8_t *buf, size_t len) { StreamSlaveClass *k = STREAM_SLAVE_GET_CLASS(sink); -return k-push(sink, buf, len, app); +return k-push(sink, buf, len); } bool diff --git a/hw/stream.h b/hw/stream.h index ff2cb14..36ae35d 100644 --- a/hw/stream.h +++ b/hw/stream.h @@ -43,12 +43,11 @@ typedef struct StreamSlaveClass { * @buf: Data to write * @len: Maximum number of bytes to write */ -size_t (*push)(StreamSlave *obj, unsigned char *buf, size_t len, -uint32_t *app); +size_t (*push)(StreamSlave *obj, unsigned char *buf, size_t len); } StreamSlaveClass; size_t -stream_push(StreamSlave *sink, uint8_t *buf, size_t len, uint32_t *app); +stream_push(StreamSlave *sink, uint8_t *buf, size_t len); bool stream_can_push(StreamSlave *sink, StreamCanPushNotifyFn notify, diff --git a/hw/xilinx.h b/hw/xilinx.h index 6c1ee21..0c0251a 100644 --- a/hw/xilinx.h +++ b/hw/xilinx.h @@ -55,16 +55,19 @@ xilinx_ethlite_create(NICInfo *nd, hwaddr base, qemu_irq irq, } static inline void -xilinx_axiethernet_init(DeviceState *dev, NICInfo *nd, StreamSlave *peer, -hwaddr base, qemu_irq irq, int txmem, int rxmem) +xilinx_axiethernet_init(DeviceState *dev, NICInfo *nd, StreamSlave *ds, +StreamSlave *cs, hwaddr base, qemu_irq irq, int txmem, +int rxmem) { Error *errp = NULL; qdev_set_nic_properties(dev, nd); qdev_prop_set_uint32(dev, rxmem, rxmem); qdev_prop_set_uint32(dev, txmem, txmem); -object_property_set_link(OBJECT(dev), OBJECT(peer), axistream-connected, - errp); +object_property_set_link(OBJECT(dev), OBJECT(ds), + axistream-connected, errp); +object_property_set_link(OBJECT(dev), OBJECT(cs), +
[Qemu-devel] [PATCH] hw/nand.c: Fix nand erase operation
From: Wendy Liang wendy.li...@xilinx.com Usually, nand erase operation has only 2 or 3 address cycles. We need to mask s-addr to zero unset stale high-order bytes in the nand address before using it as the erase address. This fixes the NAND erase operation in Linux. [PC: Generalised to work for any number of address cycles rather than just 3] Signed-off-by: Wendy Liang jli...@xilinx.com Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- hw/nand.c |1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/hw/nand.c b/hw/nand.c index de3e502..6362093 100644 --- a/hw/nand.c +++ b/hw/nand.c @@ -297,6 +297,7 @@ static void nand_command(NANDFlashState *s) break; case NAND_CMD_BLOCKERASE2: +s-addr = (1ull s-addrlen * 8) - 1; if (nand_flash_ids[s-chip_id].options NAND_SAMSUNG_LP) s-addr = 16; else -- 1.7.0.4
Re: [Qemu-devel] [PATCH 3/3] block: change rollback sequence in qmp_transaction
于 2013-4-2 21:59, Kevin Wolf 写道: Am 01.04.2013 um 12:01 hat Wenchao Xia geschrieben: Last operaton should be cancelled first. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Should it? This commit message does little to convince me. Kevin I think so, if two request are snapshot_blkdev ide0 newimage.qcow2, snapshot_blkdev_internal ide0 snap0, then in rollback delete of snap0 would be wrong. -- Best Regards Wenchao Xia
Re: [Qemu-devel] [PATCH 2/3] block: adjust qmp_transaction to be extendable
于 2013-4-2 21:55, Kevin Wolf 写道: Am 01.04.2013 um 12:01 hat Wenchao Xia geschrieben: Now code for external snapshot are packaged as one case in qmp_transaction, so later other operation could be added. The logic in qmp_transaction is changed a bit: Original code tries to create all images first and then update all *bdrv once together, new code create and update one *bdrv one time, and use bdrv_deappend() to rollback on fail. This allows mixing different kind of requests in qmp_transaction() later. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- blockdev.c | 250 +--- 1 files changed, 153 insertions(+), 97 deletions(-) diff --git a/blockdev.c b/blockdev.c index 8cdc9ce..75416fb 100644 --- a/blockdev.c +++ b/blockdev.c @@ -779,9 +779,155 @@ void qmp_blockdev_snapshot_sync(const char *device, const char *snapshot_file, /* New and old BlockDriverState structs for group snapshots */ -typedef struct BlkTransactionStates { +typedef struct BdrvActionOps { +int (*commit)(BlockdevAction *action, void **p_opaque, Error **errp); +void (*rollback)(BlockdevAction *action, void *opaque); +void (*clean)(BlockdevAction *action, void *opaque); +} BdrvActionOps; You don't really implement the transactional pattern that was used by the original code (and is used elsewhere). It should work like this: 1. Prepare: This stage performs all operations that can fail. They are not made active yet. For example with snapshotting, open a new BlockDriver state, but don't change the backing file chain yet. 2. Commit: Activate the change. This operation can never fail. For this reason, you never have to undo anything done here. 3. Rollback: Basically just free everything that prepare did up to the error. If you do it your way, you get into serious trouble if rollback involves operations that can fail. In the original code, here start the prepare: That is a clear comment, thanks. I changed the model since expecting commit operation may need rollback, if not I am confident to keep original model. Since bdrv_snapshot_delete() can fail, I guess assertion of its success should be used in the rollback later. @@ -806,125 +950,37 @@ void qmp_transaction(BlockdevActionList *dev_list, Error **errp) /* We don't do anything in this loop that commits us to the snapshot */ while (NULL != dev_entry) { BlockdevAction *dev_info = NULL; -BlockDriver *proto_drv; -BlockDriver *drv; -int flags; -enum NewImageMode mode; -const char *new_image_file; -const char *device; -const char *format = qcow2; - dev_info = dev_entry-value; dev_entry = dev_entry-next; states = g_malloc0(sizeof(BlkTransactionStates)); QSIMPLEQ_INSERT_TAIL(snap_bdrv_states, states, entry); +states-action = dev_info; switch (dev_info-kind) { case BLOCKDEV_ACTION_KIND_BLOCKDEV_SNAPSHOT_SYNC: -device = dev_info-blockdev_snapshot_sync-device; -if (!dev_info-blockdev_snapshot_sync-has_mode) { -dev_info-blockdev_snapshot_sync-mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS; -} -new_image_file = dev_info-blockdev_snapshot_sync-snapshot_file; -if (dev_info-blockdev_snapshot_sync-has_format) { -format = dev_info-blockdev_snapshot_sync-format; -} -mode = dev_info-blockdev_snapshot_sync-mode; +states-ops = external_snapshot_ops; break; default: abort(); } -drv = bdrv_find_format(format); -if (!drv) { -error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); -goto delete_and_fail; -} - -states-old_bs = bdrv_find(device); -if (!states-old_bs) { -error_set(errp, QERR_DEVICE_NOT_FOUND, device); -goto delete_and_fail; -} - -if (!bdrv_is_inserted(states-old_bs)) { -error_set(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); -goto delete_and_fail; -} - -if (bdrv_in_use(states-old_bs)) { -error_set(errp, QERR_DEVICE_IN_USE, device); -goto delete_and_fail; -} - -if (!bdrv_is_read_only(states-old_bs)) { -if (bdrv_flush(states-old_bs)) { -error_set(errp, QERR_IO_ERROR); -goto delete_and_fail; -} -} - -flags = states-old_bs-open_flags; - -proto_drv = bdrv_find_protocol(new_image_file); -if (!proto_drv) { -error_set(errp, QERR_INVALID_BLOCK_FORMAT, format); -goto delete_and_fail; -} - -/* create new image w/backing file */ -if (mode != NEW_IMAGE_MODE_EXISTING) { -bdrv_img_create(new_image_file, format, -states-old_bs-filename, -
Re: [Qemu-devel] [PATCH 2/5] hostmem: make hostmem global and RAM hotunplg safe
在 2013-04-01一的 16:20 +0800,Liu Ping Fan写道: From: Liu Ping Fan pingf...@linux.vnet.ibm.com The hostmem listener will translate gpa to hva quickly. Make it global, so other component can use it. Also this patch adopt MemoryRegionOps's ref/unref interface to make it survive the RAM hotunplug. Signed-off-by: Liu Ping Fan pingf...@linux.vnet.ibm.com --- hw/dataplane/hostmem.c | 130 +-- hw/dataplane/hostmem.h | 19 ++-- 2 files changed, 95 insertions(+), 54 deletions(-) diff --git a/hw/dataplane/hostmem.c b/hw/dataplane/hostmem.c index 380537e..86c02cd 100644 --- a/hw/dataplane/hostmem.c +++ b/hw/dataplane/hostmem.c @@ -13,6 +13,12 @@ #include exec/address-spaces.h #include hostmem.h +#include qemu/main-loop.h + +/* lock to protect the access to cur_hostmem */ +static QemuMutex hostmem_lock; +static HostMem *cur_hostmem; +static HostMem *next_hostmem; static int hostmem_lookup_cmp(const void *phys_, const void *region_) { @@ -28,16 +34,49 @@ static int hostmem_lookup_cmp(const void *phys_, const void *region_) } } +static void hostmem_ref(HostMem *hostmem) +{ +assert(__sync_add_and_fetch(hostmem-ref, 1) 0); +} + +void hostmem_unref(HostMem *hostmem) +{ +int i; +HostMemRegion *hmr; + +if (!__sync_sub_and_fetch(hostmem-ref, 1)) { maybe '__sync_sub_and_fetch(hostmem-ref, 1) 0' is more safer +for (i = 0; i hostmem-num_current_regions; i++) { +hmr = hostmem-current_regions[i]; +/* Fix me, when memory hotunplug implement + * assert(hmr-mr_ops-unref) + */ +if (hmr-mr-ops hmr-mr-ops-unref) { +hmr-mr-ops-unref(); +} +} +g_free(hostmem-current_regions); +g_free(hostmem); +} +} + /** * Map guest physical address to host pointer + * also inc refcnt of *mr, caller need to unref later */ -void *hostmem_lookup(HostMem *hostmem, hwaddr phys, hwaddr len, bool is_write) +void *hostmem_lookup(hwaddr phys, hwaddr len, MemoryRegion **mr, bool is_write) { HostMemRegion *region; void *host_addr = NULL; hwaddr offset_within_region; +HostMem *hostmem; + +assert(mr); +*mr = NULL; +qemu_mutex_lock(hostmem_lock); +hostmem = cur_hostmem; +hostmem_ref(hostmem); +qemu_mutex_unlock(hostmem_lock); -qemu_mutex_lock(hostmem-current_regions_lock); region = bsearch(phys, hostmem-current_regions, hostmem-num_current_regions, sizeof(hostmem-current_regions[0]), @@ -52,28 +91,33 @@ void *hostmem_lookup(HostMem *hostmem, hwaddr phys, hwaddr len, bool is_write) if (len = region-size - offset_within_region) { host_addr = region-host_addr + offset_within_region; } -out: -qemu_mutex_unlock(hostmem-current_regions_lock); +*mr = region-mr; +memory_region_ref(*mr); +out: +hostmem_unref(hostmem); return host_addr; } +static void hostmem_listener_begin(MemoryListener *listener) +{ +next_hostmem = g_new0(HostMem, 1); +next_hostmem-ref = 1; +} + /** * Install new regions list */ static void hostmem_listener_commit(MemoryListener *listener) { -HostMem *hostmem = container_of(listener, HostMem, listener); +HostMem *tmp; -qemu_mutex_lock(hostmem-current_regions_lock); -g_free(hostmem-current_regions); -hostmem-current_regions = hostmem-new_regions; -hostmem-num_current_regions = hostmem-num_new_regions; -qemu_mutex_unlock(hostmem-current_regions_lock); +tmp = cur_hostmem; +qemu_mutex_lock(hostmem_lock); +cur_hostmem = next_hostmem; +qemu_mutex_unlock(hostmem_lock); +hostmem_unref(tmp); -/* Reset new regions list */ -hostmem-new_regions = NULL; -hostmem-num_new_regions = 0; } /** @@ -83,23 +127,24 @@ static void hostmem_append_new_region(HostMem *hostmem, MemoryRegionSection *section) { void *ram_ptr = memory_region_get_ram_ptr(section-mr); -size_t num = hostmem-num_new_regions; -size_t new_size = (num + 1) * sizeof(hostmem-new_regions[0]); +size_t num = hostmem-num_current_regions; +size_t new_size = (num + 1) * sizeof(hostmem-current_regions[0]); -hostmem-new_regions = g_realloc(hostmem-new_regions, new_size); -hostmem-new_regions[num] = (HostMemRegion){ +hostmem-current_regions = g_realloc(hostmem-current_regions, new_size); +hostmem-current_regions[num] = (HostMemRegion){ .host_addr = ram_ptr + section-offset_within_region, .guest_addr = section-offset_within_address_space, +.mr = section-mr, .size = section-size, .readonly = section-readonly, }; -hostmem-num_new_regions++; +hostmem-num_current_regions++; } -static void
Re: [Qemu-devel] [2/2] [vmxnet3] const_cpu_to_le64 wrapping for feature bits dropped
On 28.03.2013, at 09:53, Dmitry Fleytman wrote: Byte swap is redundant because shared memory reading functions already swap bytes when required Signed-off-by: Dmitry Fleytman dmi...@daynix.com Acked-by: Alexander Graf ag...@suse.de It might be worth to run an x86 guest with vmxnet3 on a big endian host machine to find potential additional breakage. I don't think it's worth caring about big endian guests with vmxnet3 at this point. Chances are quite low the guest driver would be correct :). Alex --- hw/vmxnet3.h | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/hw/vmxnet3.h b/hw/vmxnet3.h index 7db0c8f..4eae7c7 100644 --- a/hw/vmxnet3.h +++ b/hw/vmxnet3.h @@ -37,10 +37,8 @@ #define __packed QEMU_PACKED #if defined(HOST_WORDS_BIGENDIAN) -#define const_cpu_to_le64(x) bswap_64(x) #define __BIG_ENDIAN_BITFIELD #else -#define const_cpu_to_le64(x) (x) #endif /* @@ -137,10 +135,10 @@ struct UPT1_RSSConf { /* features */ enum { -UPT1_F_RXCSUM= const_cpu_to_le64(0x0001), /* rx csum verification */ -UPT1_F_RSS= const_cpu_to_le64(0x0002), -UPT1_F_RXVLAN= const_cpu_to_le64(0x0004), /* VLAN tag stripping */ -UPT1_F_LRO= const_cpu_to_le64(0x0008), +UPT1_F_RXCSUM= 0x0001, /* rx csum verification */ +UPT1_F_RSS = 0x0002, +UPT1_F_RXVLAN= 0x0004, /* VLAN tag stripping */ +UPT1_F_LRO = 0x0008, }; /* all registers are 32 bit wide */ @@ -752,7 +750,6 @@ struct Vmxnet3_DriverShared { #undef __le32 #undef __le64 #undef __packed -#undef const_cpu_to_le64 #if defined(HOST_WORDS_BIGENDIAN) #undef __BIG_ENDIAN_BITFIELD #endif -- 1.8.1.4
Re: [Qemu-devel] [PATCH] target-ppc: Enable ISEL on POWER7
On 01.04.2013, at 17:06, Aurelien Jarno wrote: ISEL is a Power ISA 2.06 instruction and thus is available on POWER7. Given this is trapped and emulated by the Linux kernel, I guess it went unnoticed. Cc: Alexander Graf ag...@suse.de Signed-off-by: Aurelien Jarno aurel...@aurel32.net Thanks, applied to ppc-next. Alex --- target-ppc/translate_init.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 781170f..aea7d27 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -7004,7 +7004,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data) dc-desc = POWER7; pcc-init_proc = init_proc_POWER7; pcc-check_pow = check_pow_nocheck; -pcc-insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB | +pcc-insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB | PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES | PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE | PPC_FLOAT_STFIWX | -- 1.7.10.4
Re: [Qemu-devel] [PATCH v3 17/27] tcg-ppc64: Implement bswap64
On 02.04.2013, at 06:23, Richard Henderson wrote: Reviewed-by: Aurelien Jarno aurel...@aurel32.net Signed-off-by: Richard Henderson r...@twiddle.net Is this faster than a load/store with std/ldbrx? Alex --- tcg/ppc64/tcg-target.c | 35 +++ tcg/ppc64/tcg-target.h | 2 +- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c index d8131ec..1806364 100644 --- a/tcg/ppc64/tcg-target.c +++ b/tcg/ppc64/tcg-target.c @@ -1706,6 +1706,40 @@ static void tcg_out_op (TCGContext *s, TCGOpcode opc, const TCGArg *args, } break; +case INDEX_op_bswap64_i64: +a0 = args[0], a1 = args[1], a2 = 0; +if (a0 == a1) { +a0 = 0; +a2 = a1; +} + +/* a1 = # abcd efgh */ +/* a0 = rl32(a1, 8) # fghe */ +tcg_out_rlw(s, RLWINM, a0, a1, 8, 0, 31); +/* a0 = dep(a0, rl32(a1, 24), 0xff00) # hghe */ +tcg_out_rlw(s, RLWIMI, a0, a1, 24, 0, 7); +/* a0 = dep(a0, rl32(a1, 24), 0xff00) # hgfe */ +tcg_out_rlw(s, RLWIMI, a0, a1, 24, 16, 23); + +/* a0 = rl64(a0, 32) # hgfe */ +/* a2 = rl64(a1, 32) # efgh abcd */ +tcg_out_rld(s, RLDICL, a0, a0, 32, 0); +tcg_out_rld(s, RLDICL, a2, a1, 32, 0); + +/* a0 = dep(a0, rl32(a2, 8), 0x) # hgfe bcda */ +tcg_out_rlw(s, RLWIMI, a0, a2, 8, 0, 31); +/* a0 = dep(a0, rl32(a2, 24), 0xff00) # hgfe dcda */ +tcg_out_rlw(s, RLWIMI, a0, a2, 24, 0, 7); +/* a0 = dep(a0, rl32(a2, 24), 0xff00) # hgfe dcba */ +tcg_out_rlw(s, RLWIMI, a0, a2, 24, 16, 23); + +if (a0 == 0) { +tcg_out_mov(s, TCG_TYPE_I64, args[0], a0); +/* Revert the source rotate that we performed above. */ +tcg_out_rld(s, RLDICL, a1, a1, 32, 0); +} +break; + default: tcg_dump_ops (s); tcg_abort (); @@ -1816,6 +1850,7 @@ static const TCGTargetOpDef ppc_op_defs[] = { { INDEX_op_bswap16_i64, { r, r } }, { INDEX_op_bswap32_i32, { r, r } }, { INDEX_op_bswap32_i64, { r, r } }, +{ INDEX_op_bswap64_i64, { r, r } }, { -1 }, }; diff --git a/tcg/ppc64/tcg-target.h b/tcg/ppc64/tcg-target.h index bd011b8..d8e1820 100644 --- a/tcg/ppc64/tcg-target.h +++ b/tcg/ppc64/tcg-target.h @@ -102,7 +102,7 @@ typedef enum { #define TCG_TARGET_HAS_ext32u_i64 1 #define TCG_TARGET_HAS_bswap16_i64 1 #define TCG_TARGET_HAS_bswap32_i64 1 -#define TCG_TARGET_HAS_bswap64_i64 0 +#define TCG_TARGET_HAS_bswap64_i64 1 #define TCG_TARGET_HAS_not_i64 1 #define TCG_TARGET_HAS_neg_i64 1 #define TCG_TARGET_HAS_andc_i64 0 -- 1.8.1.4
Re: [Qemu-devel] [PATCH V10 12/17] hmp: add function hmp_info_snapshots()
Am 30.03.2013 um 13:38 hat Wenchao Xia geschrieben: 于 2013-3-30 7:04, Eric Blake 写道: On 03/22/2013 08:19 AM, Wenchao Xia wrote: This function will simply call qmp interface qmp_query_snapshots() added in last commit and then dump information in monitor console. To get snapshot info, Now qemu and qemu-img both call block layer function bdrv_query_snapshot_info_list() in their calling path, and then they just translate the qmp object got to strings in stdout or monitor console. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- hmp.c | 42 ++ hmp.h |1 + 2 files changed, 43 insertions(+), 0 deletions(-) diff --git a/hmp.c b/hmp.c index b0a861c..c475d65 100644 --- a/hmp.c +++ b/hmp.c @@ -651,6 +651,48 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict) qapi_free_TPMInfoList(info_list); } +/* assume list is valid */ +static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list) +{ +SnapshotInfoList *elem; +char buf[256]; + +monitor_printf(mon, %s\n, bdrv_snapshot_dump(buf, sizeof(buf), NULL)); Are you sure that won't ever be truncated? I'm pretty sure that I could come up with a scenario where I cause bdrv_snapshot_dump() to want to output more than 256 bytes of details. I hope not use dynamic buff for strings, how about check it as following: char buf[254] = 0; monitor_printf(mon, %s\n, bdrv_snapshot_dump(buf, sizeof(buf), NULL)); if (buf[254]) { monitor_printf(mon, 'string truncated'); } About as useless as the truncated string alone. That something is missing will be noticed by the user; the fix isn't to tell him that the output is broken, but to give the full output. Kevin
Re: [Qemu-devel] [PATCH V10 17/17] hmp: add parameter device and -b for info block
Am 29.03.2013 um 03:48 hat Wenchao Xia geschrieben: 于 2013-3-28 19:09, Kevin Wolf 写道: Am 22.03.2013 um 15:19 hat Wenchao Xia geschrieben: With these parameters, user can choose the information to be showed, to avoid message flood in the montior. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Aha, so here you actually introduce the 'device' parameter. If you can have this patch first, and only then patch 16, then limiting the new output to the form with a device specified should be trivial. It is a bit indirect that info block and info block device show different on single device, so I introduced additional parameter '-b' to filter out info. With more thinking, I think it should be { .name = block, .help = show the block devices, .args_type = verbose:-v,device:B?, .params = [-v] [device], .help = show info of one block device or all block devices and detail of images with -v option, } Then by default info block so brief summary as old time. Since the -v parameter is filtering something out not present on patch 15, so I can't move this patch forward, hope you are OK with it. 'info block [-v] [device]' is okay with me. I would prefer if you could arrange the patches so that without -v you never get verbose output in any intermediate patch, but otherwise I don't mind about the order of patches. Kevin
Re: [Qemu-devel] [PATCH] PPC: e500: advertise 4.2 MPIC only if KVM supports EPR
On 30.03.2013, at 17:40, Stuart Yoder wrote: From: Stuart Yoder stuart.yo...@freescale.com Missing patch description Signed-off-by: Stuart Yoder stuart.yo...@freescale.com --- hw/ppc/e500plat.c |5 + 1 file changed, 5 insertions(+) diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c index 25ac4b1..2cd7cad 100644 --- a/hw/ppc/e500plat.c +++ b/hw/ppc/e500plat.c @@ -16,6 +16,7 @@ #include sysemu/device_tree.h #include hw/pci/pci.h #include hw/openpic.h +#include sysemu/kvm.h static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt) { @@ -48,6 +49,10 @@ static void e500plat_init(QEMUMachineInitArgs *args) .mpic_version = OPENPIC_MODEL_FSL_MPIC_42, }; Missing comment +if (kvm_enabled() !kvm_check_extension(kvm_state, KVM_CAP_PPC_EPR)) { This should go through target-ppc/kvm_ppc.c. That way we don't need to have any kvm specific headers included here and guarantee that everything compiles just fine without kvm enabled. I've fixed those up for you and applied the patch to ppc-next. Alex +params.mpic_version = OPENPIC_MODEL_FSL_MPIC_20; +} + ppce500_init(params); } -- 1.7.9.7
Re: [Qemu-devel] [PATCH 1/2] PPC: Remvove env-hreset_excp_prefix
On 29.03.2013, at 13:06, Fabien Chouteau wrote: This value is not needed if we use correctly the MSR[IP] bit. excp_prefix is always 0x, except when the MSR[IP] bit is implemented and set to 1, in that case excp_prefix is 0xfff0. The handling of MSR[IP] was already implemented but not used at reset because the value of env-msr was changed manually. The patch uses the function hreg_store_msr() to set env-msr, this ensures a good handling of MSR[IP] at reset, and therefore a good value for excp_prefix. Signed-off-by: Fabien Chouteau chout...@adacore.com Thanks, applied to ppc-next. Alex --- hw/ppc/spapr.c |6 -- target-ppc/cpu.h|1 - target-ppc/machine.c|2 -- target-ppc/translate_init.c | 42 -- 4 files changed, 16 insertions(+), 35 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 7b2a11f..6a12485 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -801,8 +801,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args) /* Set time-base frequency to 512 MHz */ cpu_ppc_tb_init(env, TIMEBASE_FREQ); -/* PAPR always has exception vectors in RAM not ROM */ -env-hreset_excp_prefix = 0; +/* PAPR always has exception vectors in RAM not ROM. To ensure this, + * MSR[IP] should never be set. + */ +env-msr_mask = ~(1 6); /* Tell KVM that we're in PAPR mode */ if (kvm_enabled()) { diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h index 42c36e2..99ebf7e 100644 --- a/target-ppc/cpu.h +++ b/target-ppc/cpu.h @@ -1026,7 +1026,6 @@ struct CPUPPCState { /* Exception vectors */ target_ulong excp_vectors[POWERPC_EXCP_NB]; target_ulong excp_prefix; -target_ulong hreset_excp_prefix; target_ulong ivor_mask; target_ulong ivpr_mask; target_ulong hreset_vector; diff --git a/target-ppc/machine.c b/target-ppc/machine.c index 235b0d5..2d10adb 100644 --- a/target-ppc/machine.c +++ b/target-ppc/machine.c @@ -78,7 +78,6 @@ void cpu_save(QEMUFile *f, void *opaque) for (i = 0; i POWERPC_EXCP_NB; i++) qemu_put_betls(f, env-excp_vectors[i]); qemu_put_betls(f, env-excp_prefix); -qemu_put_betls(f, env-hreset_excp_prefix); qemu_put_betls(f, env-ivor_mask); qemu_put_betls(f, env-ivpr_mask); qemu_put_betls(f, env-hreset_vector); @@ -167,7 +166,6 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id) for (i = 0; i POWERPC_EXCP_NB; i++) qemu_get_betls(f, env-excp_vectors[i]); qemu_get_betls(f, env-excp_prefix); -qemu_get_betls(f, env-hreset_excp_prefix); qemu_get_betls(f, env-ivor_mask); qemu_get_betls(f, env-ivpr_mask); qemu_get_betls(f, env-hreset_vector); diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 781170f..ce5238b 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -2593,7 +2593,6 @@ static void init_excp_4xx_real (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_FIT] = 0x1010; env-excp_vectors[POWERPC_EXCP_WDT] = 0x1020; env-excp_vectors[POWERPC_EXCP_DEBUG]= 0x2000; -env-hreset_excp_prefix = 0xUL; env-ivor_mask = 0xFFF0UL; env-ivpr_mask = 0xUL; /* Hardware reset vector */ @@ -2618,7 +2617,6 @@ static void init_excp_4xx_softmmu (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_DTLB] = 0x1100; env-excp_vectors[POWERPC_EXCP_ITLB] = 0x1200; env-excp_vectors[POWERPC_EXCP_DEBUG]= 0x2000; -env-hreset_excp_prefix = 0xUL; env-ivor_mask = 0xFFF0UL; env-ivpr_mask = 0xUL; /* Hardware reset vector */ @@ -2644,7 +2642,6 @@ static void init_excp_MPC5xx (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_IABR] = 0x1C00; env-excp_vectors[POWERPC_EXCP_MEXTBR] = 0x1E00; env-excp_vectors[POWERPC_EXCP_NMEXTBR] = 0x1F00; -env-hreset_excp_prefix = 0xUL; env-ivor_mask = 0xFFF0UL; env-ivpr_mask = 0xUL; /* Hardware reset vector */ @@ -2676,7 +2673,6 @@ static void init_excp_MPC8xx (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_IABR] = 0x1C00; env-excp_vectors[POWERPC_EXCP_MEXTBR] = 0x1E00; env-excp_vectors[POWERPC_EXCP_NMEXTBR] = 0x1F00; -env-hreset_excp_prefix = 0xUL; env-ivor_mask = 0xFFF0UL; env-ivpr_mask = 0xUL; /* Hardware reset vector */ @@ -2704,7 +2700,6 @@ static void init_excp_G2 (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_DSTLB]= 0x1200; env-excp_vectors[POWERPC_EXCP_IABR] = 0x1300; env-excp_vectors[POWERPC_EXCP_SMI] = 0x1400; -env-hreset_excp_prefix = 0xUL; /* Hardware reset vector */ env-hreset_vector = 0xFFFCUL; #endif @@ -2733,7 +2728,6 @@ static void
Re: [Qemu-devel] [PATCH 2/2] PPC: fix hreset_vector for 60x, 7x0, 7x5, G2, MPC8xx, MPC5xx, 7400 and 7450
On 29.03.2013, at 13:06, Fabien Chouteau wrote: According to the different user's manuals, the vector offset for system reset (both /HRESET and /SRESET) is 0x00100. This patch may break support of some executables, as the power-on start address may change. For a specific board, if the power-on start address is different than HRESET vector (i.e. 0x0100 or 0xfff00100), this should be fixed in board's initialization code. Signed-off-by: Fabien Chouteau chout...@adacore.com What about the other users of hreset_vector? Can we just remove the variable altogether? Alex --- target-ppc/translate_init.c | 22 +++--- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index ce5238b..2c9b477 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -2645,7 +2645,7 @@ static void init_excp_MPC5xx (CPUPPCState *env) env-ivor_mask = 0xFFF0UL; env-ivpr_mask = 0xUL; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0x0100UL; #endif } @@ -2676,7 +2676,7 @@ static void init_excp_MPC8xx (CPUPPCState *env) env-ivor_mask = 0xFFF0UL; env-ivpr_mask = 0xUL; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0x0100UL; #endif } @@ -2701,7 +2701,7 @@ static void init_excp_G2 (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_IABR] = 0x1300; env-excp_vectors[POWERPC_EXCP_SMI] = 0x1400; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0x0100UL; #endif } @@ -2804,7 +2804,7 @@ static void init_excp_602 (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_WDT] = 0x1500; env-excp_vectors[POWERPC_EXCP_EMUL] = 0x1600; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0x0100UL; #endif } @@ -2828,7 +2828,7 @@ static void init_excp_603 (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_IABR] = 0x1300; env-excp_vectors[POWERPC_EXCP_SMI] = 0x1400; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0x0100UL; #endif } @@ -2873,7 +2873,7 @@ static void init_excp_7x0 (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_SMI] = 0x1400; env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0x0100UL; #endif } @@ -2895,7 +2895,7 @@ static void init_excp_750cl (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_IABR] = 0x1300; env-excp_vectors[POWERPC_EXCP_SMI] = 0x1400; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0x0100UL; #endif } @@ -2917,7 +2917,7 @@ static void init_excp_750cx (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_IABR] = 0x1300; env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0x0100UL; #endif } @@ -2944,7 +2944,7 @@ static void init_excp_7x5 (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_SMI] = 0x1400; env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0x0100UL; #endif } @@ -2969,7 +2969,7 @@ static void init_excp_7400 (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_VPUA] = 0x1600; env-excp_vectors[POWERPC_EXCP_THERM]= 0x1700; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0x0100UL; #endif } @@ -2996,7 +2996,7 @@ static void init_excp_7450 (CPUPPCState *env) env-excp_vectors[POWERPC_EXCP_SMI] = 0x1400; env-excp_vectors[POWERPC_EXCP_VPUA] = 0x1600; /* Hardware reset vector */ -env-hreset_vector = 0xFFFCUL; +env-hreset_vector = 0x0100UL; #endif } -- 1.7.9.5
Re: [Qemu-devel] [PATCH v2] target-ppc: fix nego and subf*o instructions
On 01.04.2013, at 02:33, Aurelien Jarno wrote: The overflow computation of nego and subf*o instructions has been broken in commit ffe30937. Contrary to other targets, the instruction is subtract from an not subtract on PowerPC. This patch fixes the issue by using the correct argument in the xor computation. Thanks to Peter Maydell for the hint. With this change the PPC emulation passes the Gwenole Beauchesne testsuite again. Cc: Alexander Graf ag...@suse.de Cc: Richard Henderson r...@twiddle.net Signed-off-by: Aurelien Jarno aurel...@aurel32.net Thanks, applied to ppc-next. Alex
Re: [Qemu-devel] [RFC PATCH 1/3] aio-context: if io_flush isn't provided, assume always busy
Am 29.03.2013 um 00:37 hat Paolo Bonzini geschrieben: Il 28/03/2013 22:52, Anthony Liguori ha scritto: Today, all callers of qemu_aio_set_fd_handler() pass a valid io_flush function. Except one: aio_set_event_notifier(ctx, ctx-notifier, (EventNotifierHandler *) event_notifier_test_and_clear, NULL); This is the EventNotifier that is used by qemu_notify_event. It's quite surprising that this patch works and passes the tests. /me reads cover letter... ah, it is untested. :) But if you can eliminate the sole usage of aio_wait()'s return value (in bdrv_drain_all()), everything would be much simpler. There is a relatively convenient assert(QLIST_EMPTY(bs-tracked_requests)); that you can use as the exit condition instead. Perhaps it's not trivial to do it efficiently, but it's not a fast path. We just need to move to .bdrv_drain() for all block driver that register an AioHandler. I'm pretty sure that each one has its own data structures to manage in-flight requests (basically what is the aio_flush handler today would become the .bdrv_drain callback). Then bdrv_drain_all() can directly use the bdrv_drain() return value and doesn't need to have it passed through aio_wait() any more. Kevin
Re: [Qemu-devel] [RFC PATCH 2/3] sheepdog: pass NULL for io_flush
Am 28.03.2013 um 22:52 hat Anthony Liguori geschrieben: Signed-off-by: Anthony Liguori aligu...@us.ibm.com --- block/sheepdog.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/block/sheepdog.c b/block/sheepdog.c index bb67c4c..2bccd9b 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -503,13 +503,6 @@ static void restart_co_req(void *opaque) qemu_coroutine_enter(co, NULL); } -static int have_co_req(void *opaque) -{ -/* this handler is set only when there is a pending request, so - * always returns 1. */ Now you return 1 even when no request is pending (which is the case in which no io_flush handler would be set before). Why is this correct? (This is actually a question about PATCH 1/3, I just noticed it here. Are there more cases like this?) Kevin
Re: [Qemu-devel] [PATCH V10 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block
Am 29.03.2013 um 03:35 hat Wenchao Xia geschrieben: 于 2013-3-28 17:54, Kevin Wolf 写道: Am 22.03.2013 um 15:19 hat Wenchao Xia geschrieben: Now image info will be retrieved as an embbed json object inside BlockDeviceInfo, backing chain info and all related internal snapshot info can be got in the enhanced recursive structure of ImageInfo. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- block/qapi.c | 39 ++-- include/block/qapi.h |4 ++- qapi-schema.json |5 +++- qmp-commands.hx | 67 +- 4 files changed, 109 insertions(+), 6 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index df73f5b..9051947 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -200,9 +200,15 @@ int bdrv_query_image_info(BlockDriverState *bs, return 0; } -BlockInfo *bdrv_query_info(BlockDriverState *bs) +/* return 0 on success, and @p_info will be set only on success. */ +int bdrv_query_info(BlockDriverState *bs, +BlockInfo **p_info, +Error **errp) { BlockInfo *info = g_malloc0(sizeof(*info)); +BlockDriverState *bs0; +ImageInfo **p_image_info; +int ret = 0; ret is never changed, so this function always returns 0. I would suggest to drop ret and make the function return type void. My bad, I forgot to set its value, the interface is intend to return negative error number and errp both on fail. Why do you need two separate error reporting mechanisms? Shouldn't only errp be enough? Kevin
Re: [Qemu-devel] [PATCH] append the terminating '\0' to bootorder string
On 03/20/13 11:16, Amos Kong wrote: Problem was introduced in commit c8a6ae8b. The last terminating '\0' was lost, use the right length 5 (HALT\0). Acked-by: Gerd Hoffmann kra...@redhat.com cheers, Gerd
Re: [Qemu-devel] [PATCH 2/2] PPC: fix hreset_vector for 60x, 7x0, 7x5, G2, MPC8xx, MPC5xx, 7400 and 7450
On 04/02/2013 10:09 AM, Alexander Graf wrote: On 29.03.2013, at 13:06, Fabien Chouteau wrote: According to the different user's manuals, the vector offset for system reset (both /HRESET and /SRESET) is 0x00100. This patch may break support of some executables, as the power-on start address may change. For a specific board, if the power-on start address is different than HRESET vector (i.e. 0x0100 or 0xfff00100), this should be fixed in board's initialization code. Signed-off-by: Fabien Chouteau chout...@adacore.com What about the other users of hreset_vector? Can we just remove the variable altogether? For the CPUs that don't implement MSR[IP], the hreset_prefix is always 0x, so hreset_vector will be the reset address (i.e. 0xFFFC) I guess all this can be converted to a CPU feature flag, telling if reset is an exception or not. -- Fabien Chouteau
Re: [Qemu-devel] [PATCH V10 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block
于 2013-4-2 16:09, Kevin Wolf 写道: Am 29.03.2013 um 03:35 hat Wenchao Xia geschrieben: 于 2013-3-28 17:54, Kevin Wolf 写道: Am 22.03.2013 um 15:19 hat Wenchao Xia geschrieben: Now image info will be retrieved as an embbed json object inside BlockDeviceInfo, backing chain info and all related internal snapshot info can be got in the enhanced recursive structure of ImageInfo. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- block/qapi.c | 39 ++-- include/block/qapi.h |4 ++- qapi-schema.json |5 +++- qmp-commands.hx | 67 +- 4 files changed, 109 insertions(+), 6 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index df73f5b..9051947 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -200,9 +200,15 @@ int bdrv_query_image_info(BlockDriverState *bs, return 0; } -BlockInfo *bdrv_query_info(BlockDriverState *bs) +/* return 0 on success, and @p_info will be set only on success. */ +int bdrv_query_info(BlockDriverState *bs, +BlockInfo **p_info, +Error **errp) { BlockInfo *info = g_malloc0(sizeof(*info)); +BlockDriverState *bs0; +ImageInfo **p_image_info; +int ret = 0; ret is never changed, so this function always returns 0. I would suggest to drop ret and make the function return type void. My bad, I forgot to set its value, the interface is intend to return negative error number and errp both on fail. Why do you need two separate error reporting mechanisms? Shouldn't only errp be enough? Kevin Returned value can tell caller what error it is, like -ENOMEDIUM. Although it is not used by caller now, but I feel better to have it just like +int bdrv_query_snapshot_info_list(BlockDriverState *bs, + SnapshotInfoList **p_list, + Error **errp) in patch 5/17. -- Best Regards Wenchao Xia
Re: [Qemu-devel] [PATCH 2/2] PPC: fix hreset_vector for 60x, 7x0, 7x5, G2, MPC8xx, MPC5xx, 7400 and 7450
On 02.04.2013, at 09:53, Fabien Chouteau wrote: On 04/02/2013 10:09 AM, Alexander Graf wrote: On 29.03.2013, at 13:06, Fabien Chouteau wrote: According to the different user's manuals, the vector offset for system reset (both /HRESET and /SRESET) is 0x00100. This patch may break support of some executables, as the power-on start address may change. For a specific board, if the power-on start address is different than HRESET vector (i.e. 0x0100 or 0xfff00100), this should be fixed in board's initialization code. Signed-off-by: Fabien Chouteau chout...@adacore.com What about the other users of hreset_vector? Can we just remove the variable altogether? For the CPUs that don't implement MSR[IP], the hreset_prefix is always 0x, so hreset_vector will be the reset address (i.e. 0xFFFC) I guess all this can be converted to a CPU feature flag, telling if reset is an exception or not. Hrm, maybe. Not sure which way is cleaner. I'll apply your patch for now. It's certainly a step into the right direction. Alex
[Qemu-devel] qemu crashed when starting vm(kvm) with vnc connect
I start a kvm VM with vnc(using the zrle protocol) connect, sometimes qemu program crashed during starting period, received signal SIGABRT. Trying about 20 times, this crash may be reproduced. I guess the cause memory corruption or double free. The backtrace shown as below: 0x7f32eda3dd95 in raise () from /lib64/libc.so.6 (gdb) bt #0 0x7f32eda3dd95 in raise () from /lib64/libc.so.6 #1 0x7f32eda3f2ab in abort () from /lib64/libc.so.6 #2 0x7f32eda77ece in __libc_message () from /lib64/libc.so.6 #3 0x7f32eda7dc06 in malloc_printerr () from /lib64/libc.so.6 #4 0x7f32eda7ecda in _int_free () from /lib64/libc.so.6 #5 0x7f32efd3452c in free_and_trace (mem=0x7f329cd0) at vl.c:2880 #6 0x7f32efd251a1 in buffer_free (buffer=0x7f32f0c82890) at ui/vnc.c:505 #7 0x7f32efd20c56 in vnc_zrle_clear (vs=0x7f32f0c762d0) at ui/vnc-enc-zrle.c:364 #8 0x7f32efd26d07 in vnc_disconnect_finish (vs=0x7f32f0c762d0) at ui/vnc.c:1050 #9 0x7f32efd275c5 in vnc_client_read (opaque=0x7f32f0c762d0) at ui/vnc.c:1349 #10 0x7f32efcb397c in qemu_iohandler_poll (readfds=0x7f32f074d020, writefds=0x7f32f074d0a0, xfds=0x7f32f074d120, ret=1) at iohandler.c:124 #11 0x7f32efcb46e8 in main_loop_wait (nonblocking=0) at main-loop.c:417 #12 0x7f32efd31159 in main_loop () at vl.c:2133 #13 0x7f32efd38070 in main (argc=46, argv=0x7fff7f5df178, envp=0x7fff7f5df2f0) at vl.c:4481
[Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread
Public bug: 1154328 Broken Commit: a29753f8aa79a34a324afebe340182a51a5aef11 ATM, the timeout from g_pollfds_fill is inhibiting unlocking of the iothread. This is capable of causing a total deadlock when hw/serial is used as a device. The bug manifests when you go -nographic -serial mon:stdio and then paste 40 or more chars into the terminal. My knowledge of this g_foo is vague at best, but my best working theory is this: - First 8 chars are recieved by the serial device no complaints. - The next 32 chars, serial returns false for can_receive() so they are buffered by the MuxDriver object - mux_chr_read() - Buffer is full, so 41st char causes false return from Muxes own can_read() - This propagates all the way up to glib_pollfds_fill and manifests as a timeout - Timeout means no unlock of IOthread. Device land never sees any more cycles so the serial port never progresses - no flushing of buffer - Deadlock Tested on petalogix_ml605 microblazeel machine model, which was faulty due to 1154328. Fix by removing the conditions on unlocking the iothread. Don't know what else this will break but the timeout is certainly the wrong condition for the unlock. Probably the real solution is to have a more selective unlock policy. I'm happy for someone to take this patch off my hands, or educate me on the correct implementation. For the peeps doing automated testing on nographic platforms this will get your build working again. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- main-loop.c |8 ++-- 1 files changed, 2 insertions(+), 6 deletions(-) diff --git a/main-loop.c b/main-loop.c index eb80ff3..a376898 100644 --- a/main-loop.c +++ b/main-loop.c @@ -194,15 +194,11 @@ static int os_host_main_loop_wait(uint32_t timeout) glib_pollfds_fill(timeout); -if (timeout 0) { -qemu_mutex_unlock_iothread(); -} +qemu_mutex_unlock_iothread(); ret = g_poll((GPollFD *)gpollfds-data, gpollfds-len, timeout); -if (timeout 0) { -qemu_mutex_lock_iothread(); -} +qemu_mutex_lock_iothread(); glib_pollfds_poll(); return ret; -- 1.7.0.4
Re: [Qemu-devel] [SeaBIOS] [PATCH v16] Add pvpanic device driver
On Mon, Apr 01, 2013 at 08:22:57PM -0400, Kevin O'Connor wrote: On Sun, Mar 31, 2013 at 05:34:10PM +0300, Gleb Natapov wrote: On Sat, Mar 30, 2013 at 09:20:09AM -0400, Kevin O'Connor wrote: On Fri, Mar 29, 2013 at 02:49:12PM +0100, Paolo Bonzini wrote: Il 29/03/2013 14:33, Kevin O'Connor ha scritto: On Fri, Mar 29, 2013 at 04:18:44PM +0800, Hu Tao wrote: pvpanic device is used to notify host(qemu) when guest panic happens. Thanks. However, we're planning a move of ACPI tables from SeaBIOS to QEMU. I think this should wait until after the move. The device should be in QEMU 1.5, and the SSDT probably will still be in SeaBIOS by then (and might even be the last to move, since it's quite complex and dynamic). I don't think it is fair to block this patch on those grounds... What is the user visible impact of not having a panic device? My main concern is that the patch creates a new fw_cfg channel between qemu and seabios thats sole purpose is to alter the OS visible ACPI tables. These types of QEMU-SeaBIOS interfaces are fragile and are (in sum) quite complex. The patch uses existing channel between qemu and seabios, one romfile_loadint() is all it takes. We already have number of interfaces to change OS visible ACPI tables, that's why we want to move ACPI table creation to QEMU in the first place. It is unfortunate to start blocking features now before we have an alternative. When ACPI table creation will move into QEMU the code in this patch will be dropped along with all the other code that serves similar purpose. Hi Gleb, If there is a general consensus that this feature is important then we'll go forward with adding it as is. To be clear though, my preference would be to go forward with moving ACPI tables into QEMU, and then add this stuff on top of that. If no one beats me to it, I'll send some initial patches myself. If we can accomplish the move before next major QEMU release we do not need this new fw_cfg file obviously. Paolo thinks this is not feasible, I haven't followed this work to close to have informed opinion. If we do merge this feature as is, it will also require a SeaBIOS release (followed by a SeaBIOS pull into QEMU). There weren't any short-term plans for a new SeaBIOS release - if this feature demands that then we need to start planning it now. It is always good to have latest BIOS with new QEMU release IMO, so lest plan it regardless. -- Gleb.
Re: [Qemu-devel] [PATCH 2/2] PPC: fix hreset_vector for 60x, 7x0, 7x5, G2, MPC8xx, MPC5xx, 7400 and 7450
On 04/02/2013 10:56 AM, Alexander Graf wrote: On 02.04.2013, at 09:53, Fabien Chouteau wrote: On 04/02/2013 10:09 AM, Alexander Graf wrote: What about the other users of hreset_vector? Can we just remove the variable altogether? For the CPUs that don't implement MSR[IP], the hreset_prefix is always 0x, so hreset_vector will be the reset address (i.e. 0xFFFC) I guess all this can be converted to a CPU feature flag, telling if reset is an exception or not. Hrm, maybe. Not sure which way is cleaner. I'll apply your patch for now. It's certainly a step into the right direction. OK, thanks, -- Fabien Chouteau
Re: [Qemu-devel] [RFC PATCH v4 00/30] ACPI memory hotplug
On Fri, Jan 11, 2013 at 2:57 AM, Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com wrote: IIRC q35 supports memory hotplug natively (picked up in some discussion). Is that correct? From previous discussion I also understand that q35 supports native hotplug. Sections 5.1 and 5.2 of the spec describe the MCH registers but the native memory hotplug specifics are not yet clear to me. Any pointers from the spec are welcome. Ping. Could anyone who's familiar with the q35 spec provide some pointers on native memory hotplug details in the spec? I see pcie hotplug registers but can't find memory hotplug interface details. If I am not mistaken, the spec is here: http://www.intel.com/design/chipsets/datashts/316966.htm Is the q35 memory hotplug support supposed to be an shpc-like interface geared towards memory slots instead of pci slots? I think there is no dedicated pci express downstream port for memory slots, so native plug is not available. thanks, - Vasilis
Re: [Qemu-devel] [PATCH 2/2] Monitor: Make output buffer dynamic
On 03/27/13 07:45, Wenchao Xia wrote: Hi, Luiz Personally I hope reduce the dynamic allocated buffer which brings fragments and unexpected memory grow. It's a tradeoff. We can reduce the dynamic allocation, by simply reusing the qstring instead of allocation a new one (after complete/partial write). At the cost of wasting some memory. Given that qstrings grow expotentially I think the dynamic allocation overhead isn't that bad. Instead, how about sacrifice some time to wait output complete, since monitor is not time critical? The watch added recently will fire when there is space for writing. Right now it will write out the next chunk from the buffer. In theory we could change the monitor code to continue generating the response when there is more output space, so we don't have to buffer it in the first place. But I think the complexity simply isn't worth it for the memory savings we can gain. cheers, Gerd
Re: [Qemu-devel] [PATCH 0/2] Monitor: make output buffer dynamic
On 03/25/13 20:40, Luiz Capitulino wrote: This series fixes an easy to reproduce assertion in the Monitor code. The second patch contains all the relevant details. Gerd, I'd like a reviewed-by from you before merging this. Series looks good to me. Reviewed-by: Gerd Hoffmann kra...@redhat.com cheers, Gerd
[Qemu-devel] [PATCH 1/2] po/Makefile: Fix dependency for %.mo
Otherwise make will refuse to build updated .po files. Signed-off-by: Kevin Wolf kw...@redhat.com --- po/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/po/Makefile b/po/Makefile index 8297ab5..60ccd7d 100644 --- a/po/Makefile +++ b/po/Makefile @@ -30,7 +30,7 @@ install: $(OBJS) $(INSTALL) -m644 $$obj $(DESTDIR)$(prefix)/share/locale/$$base/LC_MESSAGES/qemu.mo; \ done -%.mo: +%.mo: %.po @msgfmt -o $@ $(SRC_PATH)/po/`basename $@ .mo`.po messages.po: $(SRC_PATH)/ui/gtk.c -- 1.8.1.4
[Qemu-devel] [PATCH 0/2] More translation improvements
This series is meant to be applied on top of Aurélien's series. Kevin Wolf (2): po/Makefile: Fix dependency for %.mo po: Update German translation po/Makefile | 2 +- po/de_DE.po | 12 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) -- 1.8.1.4
[Qemu-devel] [PATCH 2/2] po: Update German translation
Signed-off-by: Kevin Wolf kw...@redhat.com --- po/de_DE.po | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/po/de_DE.po b/po/de_DE.po index 2566674..92c5df5 100644 --- a/po/de_DE.po +++ b/po/de_DE.po @@ -18,27 +18,27 @@ msgstr #: ../ui/gtk.c:213 msgid - Press Ctrl+Alt+G to release grab -msgstr +msgstr - Strg+Alt+G drücken, um Eingabegeräte freizugeben #: ../ui/gtk.c:217 msgid [Paused] -msgstr +msgstr [Angehalten] #: ../ui/gtk.c:1250 msgid _Machine -msgstr +msgstr _Maschine #: ../ui/gtk.c:1252 msgid _Pause -msgstr +msgstr _Angehalten #: ../ui/gtk.c:1258 msgid _Reset -msgstr +msgstr _Reset #: ../ui/gtk.c:1261 msgid Power _Down -msgstr +msgstr _Herunterfahren #: ../ui/gtk.c:1276 msgid _View -- 1.8.1.4
Re: [Qemu-devel] [PATCH V10 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block
Am 02.04.2013 um 10:54 hat Wenchao Xia geschrieben: 于 2013-4-2 16:09, Kevin Wolf 写道: Am 29.03.2013 um 03:35 hat Wenchao Xia geschrieben: 于 2013-3-28 17:54, Kevin Wolf 写道: Am 22.03.2013 um 15:19 hat Wenchao Xia geschrieben: Now image info will be retrieved as an embbed json object inside BlockDeviceInfo, backing chain info and all related internal snapshot info can be got in the enhanced recursive structure of ImageInfo. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- block/qapi.c | 39 ++-- include/block/qapi.h |4 ++- qapi-schema.json |5 +++- qmp-commands.hx | 67 +- 4 files changed, 109 insertions(+), 6 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index df73f5b..9051947 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -200,9 +200,15 @@ int bdrv_query_image_info(BlockDriverState *bs, return 0; } -BlockInfo *bdrv_query_info(BlockDriverState *bs) +/* return 0 on success, and @p_info will be set only on success. */ +int bdrv_query_info(BlockDriverState *bs, +BlockInfo **p_info, +Error **errp) { BlockInfo *info = g_malloc0(sizeof(*info)); +BlockDriverState *bs0; +ImageInfo **p_image_info; +int ret = 0; ret is never changed, so this function always returns 0. I would suggest to drop ret and make the function return type void. My bad, I forgot to set its value, the interface is intend to return negative error number and errp both on fail. Why do you need two separate error reporting mechanisms? Shouldn't only errp be enough? Kevin Returned value can tell caller what error it is, like -ENOMEDIUM. Although it is not used by caller now, but I feel better to have it No, let's remove it if there is no user. We can always add it back if we ever need it. I doubt that we will. Kevin
Re: [Qemu-devel] [RFC PATCH v2 0/4] port network layer onto glib
On Thu, Mar 28, 2013 at 9:40 PM, Stefan Hajnoczi stefa...@gmail.com wrote: On Thu, Mar 28, 2013 at 09:42:47AM +0100, Paolo Bonzini wrote: Il 28/03/2013 08:55, Liu Ping Fan ha scritto: 3rd. block layer's AioContext will block other AioContexts on the same thread. I cannot understand this. The plan is for BlockDriverState to be bound to an AioContext. That means each thread is set up with one AioContext. BlockDriverStates that are used in that thread will first be bound to its AioContext. It's not very useful to have multiple AioContext in the same thread. But it can be the case that we detach and re-attach the different device( AioContext) to the same thread. I think that the design of io_flush is to sync, but for NetClientState, we need something else. So if we use AioContext, is it proper to extend readable/writeable interface for qemu_aio_set_fd_handler()? Thanks, Pingfan Stefan
Re: [Qemu-devel] [PATCH] acpi: initialize s4_val used in s4 shutdown
On 04/01/13 18:57, Bruce Rogers wrote: While investigating why a 32 bit Windows 2003 guest wasn't able to successfully perform a shutdown /h, it was discovered that commit afafe4bbe0cf7d3318e1ac7b40925561f86a6bd4 inadvertently dropped the initialization of the s4_val used to handle s4 shutdown. Initialize the value as before. --- a/hw/acpi.c +++ b/hw/acpi.c @@ -474,6 +474,7 @@ static const MemoryRegionOps acpi_pm_cnt_ops = { void acpi_pm1_cnt_init(ACPIREGS *ar, MemoryRegion *parent) { +ar-pm1.cnt.s4_val = 2; The '2' used to come from PIIX4PMState-s4_val before commit afafe4bbe0cf7d3318e1ac7b40925561f86a6bd4, see piix4_pm_properties, this behavior should be maintained IMHO. cheers, Gerd
Re: [Qemu-devel] [RFC PATCH 2/3] sheepdog: pass NULL for io_flush
diff --git a/block/sheepdog.c b/block/sheepdog.c index bb67c4c..2bccd9b 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -503,13 +503,6 @@ static void restart_co_req(void *opaque) qemu_coroutine_enter(co, NULL); } -static int have_co_req(void *opaque) -{ -/* this handler is set only when there is a pending request, so - * always returns 1. */ Now you return 1 even when no request is pending (which is the case in which no io_flush handler would be set before). Why is this correct? (This is actually a question about PATCH 1/3, I just noticed it here. Are there more cases like this?) In the dataplane code, the ioeventfd uses an io_flush callback that returns true. Paolo
Re: [Qemu-devel] [PATCH 1/1] rng backend: open backend in blocking mode
On (Mon) 01 Apr 2013 [09:02:46], Anthony Liguori wrote: Amit Shah amit.s...@redhat.com writes: Opening backends in non-blocking mode isn't necessary, we don't do anything while waiting for data. This also excuses us from checking for EAGAIN, which for the default random backend, is a very common return error type. It's not common... It really shouldn't happen however. EAGAIN is common when a file is opened in non-blocking mode. Needs to be made verbose? Starting the guest with '-device virtio-rng-pci', issuing a 'cat /dev/hwrng' in the guest while also doing 'cat /dev/random' on the host causes You are essentially cat'ing the same device twice. What's happening is that there is entropy available in /dev/random so a select() notification happens but before we are able to read() it, the cat of /dev/hwrng ends up consuming that entropy. This would never happen with a socket, for instance. /dev/random is special in there are multiple readers. backends/rng-random.c:44:entropy_available: assertion failed: (len != -1) without this fix. This fix would cause QEMU to block indefinitely which I don't think is very good behavior. I think a better solution would be: diff --git a/backends/rng-random.c b/backends/rng-random.c index acd20af..9fde566 100644 --- a/backends/rng-random.c +++ b/backends/rng-random.c @@ -41,6 +41,9 @@ static void entropy_available(void *opaque) ssize_t len; len = read(s-fd, buffer, s-size); +if (len == -1 errno == EINTR) { +return; +} This has to be an additional fix on top of this one. EAGAIN has to be handled if we want to allow nonblocking reads, and there doesn't seem to be any reason to have these reads be non-blocking. OTOH, I also think we could use the glib functions for file IO, since handling EINTR in each open-coded read call isn't always fun. Amit
Re: [Qemu-devel] [PATCH 1/7] virtio-console: Also throttle when less was written then requested
On (Mon) 01 Apr 2013 [18:00:55], Hans de Goede wrote: Hi, On 03/29/2013 10:31 AM, Amit Shah wrote: On (Thu) 28 Mar 2013 [14:28:11], Hans de Goede wrote: This is necessary so that we get properly woken up to write the rest. Signed-off-by: Hans de Goede hdego...@redhat.com Acked-by: Amit Shah amit.s...@redhat.com --- hw/virtio-console.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/hw/virtio-console.c b/hw/virtio-console.c index 284180f..015947c 100644 --- a/hw/virtio-console.c +++ b/hw/virtio-console.c @@ -47,7 +47,7 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const uint8_t *buf, size_t len) ret = qemu_chr_fe_write(vcon-chr, buf, len); trace_virtio_console_flush_buf(port-id, len, ret); -if (ret = 0) { +if (ret len) { Oops, there's a conversion bug here: len is size_t, and ret is ssize_t. Both need to be the same type. Since we're not returning negative, ret should be changed to size_t. The function signature changes too, but that's alright. Erm changing ret to a size_t will break things, since qemu_chr_fe_write can return 0 and storing that into an unsigned will change it into a very large value instead ... Oh, definitely. I meant changing the return type of flush_buf(), but that doesn't help matters much... A better fix would be to change len to ssize_t. Although that will cause issues if we ever get a single buf which is larger then 2^31 - 1. Note we already have that problem since qemu_chr_fe_write already cannot handle such large buffers... Yep. Do you want to do that as part of this series? Amit
[Qemu-devel] [PATCH] vga.c: Debug messages go to stderr
From: Alexandru DAMIAN alexandru.dam...@intel.com Move debug messages to stderr since often in vga code debug stdio is used as serial console. Signed-off-by: Alexandru DAMIAN alexandru.dam...@intel.com --- hw/vga.c | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/hw/vga.c b/hw/vga.c index 59bfb22..2e337b8 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -391,7 +391,7 @@ uint32_t vga_ioport_read(void *opaque, uint32_t addr) case VGA_SEQ_D: val = s-sr[s-sr_index]; #ifdef DEBUG_VGA_REG -printf(vga: read SR%x = 0x%02x\n, s-sr_index, val); +fprintf(stderr, vga: read SR%x = 0x%02x\n, s-sr_index, val); #endif break; case VGA_PEL_IR: @@ -419,7 +419,7 @@ uint32_t vga_ioport_read(void *opaque, uint32_t addr) case VGA_GFX_D: val = s-gr[s-gr_index]; #ifdef DEBUG_VGA_REG -printf(vga: read GR%x = 0x%02x\n, s-gr_index, val); +fprintf(stderr, vga: read GR%x = 0x%02x\n, s-gr_index, val); #endif break; case VGA_CRT_IM: @@ -430,7 +430,7 @@ uint32_t vga_ioport_read(void *opaque, uint32_t addr) case VGA_CRT_DC: val = s-cr[s-cr_index]; #ifdef DEBUG_VGA_REG -printf(vga: read CR%x = 0x%02x\n, s-cr_index, val); +fprintf(stderr, vga: read CR%x = 0x%02x\n, s-cr_index, val); #endif break; case VGA_IS1_RM: @@ -445,7 +445,7 @@ uint32_t vga_ioport_read(void *opaque, uint32_t addr) } } #if defined(DEBUG_VGA) -printf(VGA: read addr=0x%04x data=0x%02x\n, addr, val); +fprintf(stderr, VGA: read addr=0x%04x data=0x%02x\n, addr, val); #endif return val; } @@ -462,7 +462,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val) return; } #ifdef DEBUG_VGA -printf(VGA: write addr=0x%04x data=0x%02x\n, addr, val); +fprintf(stderr, VGA: write addr=0x%04x data=0x%02x\n, addr, val); #endif switch(addr) { @@ -506,7 +506,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val) break; case VGA_SEQ_D: #ifdef DEBUG_VGA_REG -printf(vga: write SR%x = 0x%02x\n, s-sr_index, val); +fprintf(stderr, vga: write SR%x = 0x%02x\n, s-sr_index, val); #endif s-sr[s-sr_index] = val sr_mask[s-sr_index]; if (s-sr_index == VGA_SEQ_CLOCK_MODE) { @@ -537,7 +537,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val) break; case VGA_GFX_D: #ifdef DEBUG_VGA_REG -printf(vga: write GR%x = 0x%02x\n, s-gr_index, val); +fprintf(stderr, vga: write GR%x = 0x%02x\n, s-gr_index, val); #endif s-gr[s-gr_index] = val gr_mask[s-gr_index]; vga_update_memory_access(s); @@ -549,7 +549,7 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val) case VGA_CRT_DM: case VGA_CRT_DC: #ifdef DEBUG_VGA_REG -printf(vga: write CR%x = 0x%02x\n, s-cr_index, val); +fprintf(stderr, vga: write CR%x = 0x%02x\n, s-cr_index, val); #endif /* handle CR0-7 protection */ if ((s-cr[VGA_CRTC_V_SYNC_END] VGA_CR11_LOCK_CR0_CR7) @@ -848,7 +848,7 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val) uint32_t write_mask, bit_mask, set_mask; #ifdef DEBUG_VGA_MEM -printf(vga: [0x TARGET_FMT_plx ] = 0x%02x\n, addr, val); +fprintf(stderr, vga: [0x TARGET_FMT_plx ] = 0x%02x\n, addr, val); #endif /* convert to VGA memory offset */ memory_map_mode = (s-gr[VGA_GFX_MISC] 2) 3; @@ -881,7 +881,7 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val) if (s-sr[VGA_SEQ_PLANE_WRITE] mask) { s-vram_ptr[addr] = val; #ifdef DEBUG_VGA_MEM -printf(vga: chain4: [0x TARGET_FMT_plx ]\n, addr); +fprintf(stderr, vga: chain4: [0x TARGET_FMT_plx ]\n, addr); #endif s-plane_updated |= mask; /* only used to detect font change */ memory_region_set_dirty(s-vram, addr, 1); @@ -894,7 +894,7 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val) addr = ((addr ~1) 1) | plane; s-vram_ptr[addr] = val; #ifdef DEBUG_VGA_MEM -printf(vga: odd/even: [0x TARGET_FMT_plx ]\n, addr); +fprintf(stderr, vga: odd/even: [0x TARGET_FMT_plx ]\n, addr); #endif s-plane_updated |= mask; /* only used to detect font change */ memory_region_set_dirty(s-vram, addr, 1); @@ -969,7 +969,7 @@ void vga_mem_writeb(VGACommonState *s, hwaddr addr, uint32_t val) (((uint32_t *)s-vram_ptr)[addr] ~write_mask) | (val write_mask); #ifdef DEBUG_VGA_MEM -printf(vga: latch: [0x TARGET_FMT_plx ] mask=0x%08x val=0x%08x\n, +fprintf(stderr, vga: latch: [0x TARGET_FMT_plx ] mask=0x%08x val=0x%08x\n, addr * 4, write_mask, val); #endif memory_region_set_dirty(s-vram, addr 2, sizeof(uint32_t)); --
Re: [Qemu-devel] [PATCH] [RFC] Xen PV backend: Move call to bdrv_new from blk_init to blk_connect
On Mon, 1 Apr 2013, Alex Bligh wrote: Stefano, --On 1 April 2013 16:44:05 +0100 Stefano Stabellini stefano.stabell...@eu.citrix.com wrote: Note this patch is compile-tested only. I think the patch looks good, just a minor comment. Thanks. I guess I ought to actually test it works then :-) +/* fill info + * Temporarily write zero sectors as we won't know file size until + * bdrv_new has been called. blk_connect corrects this. + */ +xenstore_write_be_int(blkdev-xendev, feature-flush-cache, 1); +xenstore_write_be_int(blkdev-xendev, feature-persistent, 1); +xenstore_write_be_int(blkdev-xendev, info, info); +xenstore_write_be_int(blkdev-xendev, sector-size, BLOCK_SIZE); +xenstore_write_be_int(blkdev-xendev, sectors, 0); +return 0; There is no need to fill the sector-size and sectors info here, you can do it later in blk_connect. My concern (not knowing how xenstore works) was the possibility of leaving them as uninitialized values. I'm taking it that if I don't initialise them, the key is just absent - correct? Right I'll redo and move setting sector-size and sectors into blk_connect, and then add a second commit which removes BDRV_O_NOCACHE. Oh, and test it ... Good :)
Re: [Qemu-devel] [RFC PATCH] main-loop: Unconditionally unlock iothread
Il 02/04/2013 11:04, Peter Crosthwaite ha scritto: Public bug: 1154328 Broken Commit: a29753f8aa79a34a324afebe340182a51a5aef11 ATM, the timeout from g_pollfds_fill is inhibiting unlocking of the iothread. This is capable of causing a total deadlock when hw/serial is used as a device. The bug manifests when you go -nographic -serial mon:stdio and then paste 40 or more chars into the terminal. My knowledge of this g_foo is vague at best, but my best working theory is this: - First 8 chars are recieved by the serial device no complaints. - The next 32 chars, serial returns false for can_receive() so they are buffered by the MuxDriver object - mux_chr_read() - Buffer is full, so 41st char causes false return from Muxes own can_read() - This propagates all the way up to glib_pollfds_fill and manifests as a timeout I suppose you mean manifests as timeout==0. The question is *which* GSource has a timeout of zero? Not the mux's: if mux_chr_can_read() returns zero, the prepare function returns FALSE without touching the timeout at all... static gboolean io_watch_poll_prepare(GSource *source, gint *timeout_) { IOWatchPoll *iwp = io_watch_poll_from_source(source); iwp-max_size = iwp-fd_can_read(iwp-opaque); if (iwp-max_size == 0) { return FALSE; } return g_io_watch_funcs.prepare(source, timeout_); } - Timeout means no unlock of IOthread. Device land never sees any more cycles so the serial port never progresses - no flushing of buffer Still, this is plausible, so the patch looks correct. Paolo - Deadlock Tested on petalogix_ml605 microblazeel machine model, which was faulty due to 1154328. Fix by removing the conditions on unlocking the iothread. Don't know what else this will break but the timeout is certainly the wrong condition for the unlock. Probably the real solution is to have a more selective unlock policy. I'm happy for someone to take this patch off my hands, or educate me on the correct implementation. For the peeps doing automated testing on nographic platforms this will get your build working again. Signed-off-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- main-loop.c |8 ++-- 1 files changed, 2 insertions(+), 6 deletions(-) diff --git a/main-loop.c b/main-loop.c index eb80ff3..a376898 100644 --- a/main-loop.c +++ b/main-loop.c @@ -194,15 +194,11 @@ static int os_host_main_loop_wait(uint32_t timeout) glib_pollfds_fill(timeout); -if (timeout 0) { -qemu_mutex_unlock_iothread(); -} +qemu_mutex_unlock_iothread(); ret = g_poll((GPollFD *)gpollfds-data, gpollfds-len, timeout); -if (timeout 0) { -qemu_mutex_lock_iothread(); -} +qemu_mutex_lock_iothread(); glib_pollfds_poll(); return ret;
Re: [Qemu-devel] [PATCH 13/16] IPMI: Add an external connection simulation interface
What's the status of the IPMI patch? When will be merged into qemu? With this patch, an external watchdog can be used for VM HA, even through qemu is not healthy. This is more attractive to qemu's own watchdog (ib700 or 6300esb). On 2012/9/19 4:00, miny...@acm.org wrote: From: Corey Minyard cminy...@mvista.com This adds an interface for IPMI that connects to a remote BMC over a chardev (generally a TCP socket). The OpenIPMI lanserv simulator describes this interface, see that for interface details. Signed-off-by: Corey Minyard cminy...@mvista.com --- default-configs/i386-softmmu.mak |1 + default-configs/x86_64-softmmu.mak |1 + hw/Makefile.objs |1 + hw/ipmi_extern.c | 475 4 files changed, 478 insertions(+), 0 deletions(-) create mode 100644 hw/ipmi_extern.c diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 8c99d5d..325f92e 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -12,6 +12,7 @@ CONFIG_ISA_IPMI=y CONFIG_IPMI_KCS=y CONFIG_IPMI_BT=y CONFIG_IPMI_LOCAL=y +CONFIG_IPMI_EXTERN=y CONFIG_SERIAL=y CONFIG_PARALLEL=y CONFIG_I8254=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 4d01883..2ac9177 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -12,6 +12,7 @@ CONFIG_ISA_IPMI=y CONFIG_IPMI_KCS=y CONFIG_IPMI_BT=y CONFIG_IPMI_LOCAL=y +CONFIG_IPMI_EXTERN=y CONFIG_SERIAL=y CONFIG_PARALLEL=y CONFIG_I8254=y diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 65f0ea1..a3f14ff 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -25,6 +25,7 @@ hw-obj-$(CONFIG_ISA_IPMI) += isa_ipmi.o hw-obj-$(CONFIG_IPMI_KCS) += ipmi_kcs.o hw-obj-$(CONFIG_IPMI_BT) += ipmi_bt.o hw-obj-$(CONFIG_IPMI_LOCAL) += ipmi_sim.o +hw-obj-$(CONFIG_IPMI_EXTERN) += ipmi_extern.o hw-obj-$(CONFIG_SERIAL) += serial.o hw-obj-$(CONFIG_PARALLEL) += parallel.o diff --git a/hw/ipmi_extern.c b/hw/ipmi_extern.c new file mode 100644 index 000..b31aea0 --- /dev/null +++ b/hw/ipmi_extern.c @@ -0,0 +1,475 @@ +/* + * IPMI BMC external connection + * + * Copyright (c) 2012 Corey Minyard, MontaVista Software, LLC + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +/* + * This is designed to connect with OpenIPMI's lanserv serial interface + * using the VM connection type. See that for details. + */ + +#include stdint.h +#include qemu-timer.h +#include qemu-char.h +#include ipmi.h + +#define VM_MSG_CHAR0xA0 /* Marks end of message */ +#define VM_CMD_CHAR0xA1 /* Marks end of a command */ +#define VM_ESCAPE_CHAR 0xAA /* Set bit 4 from the next byte to 0 */ + +#define VM_PROTOCOL_VERSION1 +#define VM_CMD_VERSION 0xff /* A version number byte follows */ +#define VM_CMD_NOATTN 0x00 +#define VM_CMD_ATTN0x01 +#define VM_CMD_ATTN_IRQ0x02 +#define VM_CMD_POWEROFF0x03 +#define VM_CMD_RESET 0x04 +#define VM_CMD_ENABLE_IRQ 0x05 /* Enable/disable the messaging irq */ +#define VM_CMD_DISABLE_IRQ 0x06 +#define VM_CMD_SEND_NMI0x07 +#define VM_CMD_CAPABILITIES0x08 +#define VM_CAPABILITIES_POWER0x01 +#define VM_CAPABILITIES_RESET0x02 +#define VM_CAPABILITIES_IRQ 0x04 +#define VM_CAPABILITIES_NMI 0x08 +#define VM_CAPABILITIES_ATTN 0x10 + +#define IPMI_BMC_EXTERN(obj) OBJECT_CHECK(IPMIExternBmc, (obj), \ +TYPE_IPMI_BMC_EXTERN) +typedef struct IPMIExternBmc { +IPMIBmc parent; + +int connected; +int is_listen; + +unsigned char inbuf[MAX_IPMI_MSG_SIZE + 2]; +unsigned int inpos; +int in_escape; +int
[Qemu-devel] [PATCH 0/2 V3] virtio-spec/net: dynamic network offloads configuration
From: Dmitry Fleytman dfley...@redhat.com V3 changes: 1. Compat macro added 2. Feature name beautification V2 changes: 1. _GUEST_ added to command and feature names 2. Live migration logic fixed Reported-by: Michael S. Tsirkin m...@redhat.com One of recently introduced Windows features (RSC) requires network driver to be able to enable and disable HW LRO offload on the fly without device reinitialization. Current Virtio specification doesn't support this requirement. The solution proposed by following spec patch is to add a new control command for this purpose. The same solution may be used in Linux driver for ethtool interface implementation. Patch for QEMU mainline that implements this specification change attached as well. -- 1.8.1.4
[Qemu-devel] [PATCH 1/2 V3] virtio-spec: dynamic network offloads configuration
From: Dmitry Fleytman dfley...@redhat.com Virtio-net driver currently negotiates network offloads on startup via features mechanism and have no ability to change offloads state later. This patch introduced a new control command that allows to configure device network offloads state dynamically. The patch also introduces a new feature flag VIRTIO_NET_F_CTRL_GUEST_OFFLOADS. Signed-off-by: Dmitry Fleytman dfley...@redhat.com --- virtio-spec.lyx | 143 1 file changed, 143 insertions(+) diff --git a/virtio-spec.lyx b/virtio-spec.lyx index 3d2f485..fdba814 100644 --- a/virtio-spec.lyx +++ b/virtio-spec.lyx @@ -60,6 +60,7 @@ \author -1930653948 Amos Kong \author -608949062 Rusty Russell,,, \author -385801441 Cornelia Huck cornelia.h...@de.ibm.com +\author 460276516 Dmitry Fleytman dfley...@redhat.com \author 1112500848 Rusty Russell ru...@rustcorp.com.au \author 1531152142 Paolo Bonzini,,, \author 1717892615 Alexey Zaytsev,,, @@ -4261,6 +4262,20 @@ VIRTIO_NET_F_GUEST_CSUM \end_inset (1) Guest handles packets with partial checksum +\change_inserted 460276516 1363712169 + +\end_layout + +\begin_layout Description + +\change_inserted 460276516 1363712334 +VIRTIO_NET_F_CTRL_GUEST_OFFLOADS +\begin_inset space ~ +\end_inset + +(2) Control channel offloads reconfiguration support. +\change_unchanged + \end_layout \begin_layout Description @@ -5675,6 +5690,134 @@ virtqueue_pairs = 1 \end_layout +\begin_layout Subsection* + +\change_inserted 460276516 1363765850 +Offloads State Configuration +\end_layout + +\begin_layout Standard + +\change_inserted 460276516 1363765861 +If the VIRTIO_NET_F_CTRL_GUEST_OFFLOADS feature is negotiated, the driver can + send control commands for dynamic offloads state configuration. +\end_layout + +\begin_layout Subsubsection* + +\change_inserted 460276516 1363765928 +Setting offloads state +\end_layout + +\begin_layout Standard + +\change_inserted 460276516 1363713225 +\begin_inset listings +inline false +status open + +\begin_layout Plain Layout + +\change_inserted 460276516 1363765996 + +u32 offloads; +\end_layout + +\begin_layout Plain Layout + +\change_inserted 460276516 1363765997 + +\end_layout + +\begin_layout Plain Layout + +\change_inserted 460276516 1363766044 + +#define VIRTIO_NET_OFFLOAD_GUEST_CSUM 1 +\end_layout + +\begin_layout Plain Layout + +\change_inserted 460276516 1363766051 + +#define VIRTIO_NET_OFFLOAD_GUEST_TSO4 2 +\end_layout + +\begin_layout Plain Layout + +\change_inserted 460276516 1363766055 + +#define VIRTIO_NET_OFFLOAD_GUEST_TSO6 4 +\end_layout + +\begin_layout Plain Layout + +\change_inserted 460276516 1363766064 + +#define VIRTIO_NET_OFFLOAD_GUEST_ECN8 +\end_layout + +\begin_layout Plain Layout + +\change_inserted 460276516 1363766035 + +#define VIRTIO_NET_OFFLOAD_GUEST_UFO 16 +\end_layout + +\begin_layout Plain Layout + +\change_inserted 460276516 1363766031 + +\end_layout + +\begin_layout Plain Layout + +\change_inserted 460276516 1363765865 + +#define VIRTIO_NET_CTRL_GUEST_OFFLOADS 5 +\end_layout + +\begin_layout Plain Layout + +\change_inserted 460276516 1363765867 + + #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET 0 +\end_layout + +\end_inset + + +\end_layout + +\begin_layout Standard + +\change_inserted 460276516 1363766082 +The class VIRTIO_NET_CTRL_GUEST_OFFLOADS has one command: VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET + applies the new offloads configuration. +\end_layout + +\begin_layout Standard + +\change_inserted 460276516 1363766435 +u32 value passed as command data is a bitmask, bits set define offloads + to be enabled, bits cleared - offloads to be disabled. +\end_layout + +\begin_layout Standard + +\change_inserted 460276516 1363766757 +There is a corresponding device feature for each offload. + Upon feature negotiation corresponding offload gets enabled to preserve + backward compartibility. +\end_layout + +\begin_layout Standard + +\change_inserted 460276516 1363766720 +Corresponding feature must be negotiated at startup in order to allow dynamic + change of specific offload state. +\end_layout + \begin_layout Chapter* Appendix D: Block Device \end_layout -- 1.8.1.4
[Qemu-devel] [PATCH 2/2 V3] virtio-net: dynamic network offloads configuration
From: Dmitry Fleytman dfley...@redhat.com Virtio-net driver currently negotiates network offloads on startup via features mechanism and have no ability to change offloads state later. This patch introduced a new control command that allows to configure device network offloads state dynamically. The patch also introduces a new feature flag VIRTIO_NET_F_CTRL_GUEST_OFFLOADS. Signed-off-by: Dmitry Fleytman dfley...@redhat.com --- hw/pc.h | 4 ++ hw/virtio-net.c | 111 hw/virtio-net.h | 19 ++ 3 files changed, 119 insertions(+), 15 deletions(-) diff --git a/hw/pc.h b/hw/pc.h index 8e1dd4c..7ca4698 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -221,6 +221,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); .property = vectors,\ /* DEV_NVECTORS_UNSPECIFIED as a uint32_t string */\ .value= stringify(0x),\ +},{ \ +.driver = virtio-net-pci, \ +.property = ctrl_guest_offloads, \ +.value= off, \ },{\ .driver = e1000,\ .property = romfile,\ diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 5917740..d040f96 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -360,6 +360,48 @@ static uint32_t virtio_net_bad_features(VirtIODevice *vdev) return features; } +static void virtio_net_apply_offloads(VirtIONet *n) +{ +tap_set_offload(qemu_get_subqueue(n-nic, 0)-peer, +!!(n-curr_offloads VIRTIO_NET_OFFLOAD_GUEST_CSUM), +!!(n-curr_offloads VIRTIO_NET_OFFLOAD_GUEST_TSO4), +!!(n-curr_offloads VIRTIO_NET_OFFLOAD_GUEST_TSO6), +!!(n-curr_offloads VIRTIO_NET_OFFLOAD_GUEST_ECN), +!!(n-curr_offloads VIRTIO_NET_OFFLOAD_GUEST_UFO)); +} + +static uint32_t virtio_net_offloads_by_features(uint32_t features) +{ +uint32_t offloads = 0; + +if ((1 VIRTIO_NET_F_GUEST_CSUM) features) { +offloads |= VIRTIO_NET_OFFLOAD_GUEST_CSUM; +} + +if ((1 VIRTIO_NET_F_GUEST_TSO4) features) { +offloads |= VIRTIO_NET_OFFLOAD_GUEST_TSO4; +} + +if ((1 VIRTIO_NET_F_GUEST_TSO6) features) { +offloads |= VIRTIO_NET_OFFLOAD_GUEST_TSO6; +} + +if ((1 VIRTIO_NET_F_GUEST_ECN) features) { +offloads |= VIRTIO_NET_OFFLOAD_GUEST_ECN; +} + +if ((1 VIRTIO_NET_F_GUEST_UFO) features) { +offloads |= VIRTIO_NET_OFFLOAD_GUEST_UFO; +} + +return offloads; +} + +static inline uint32_t virtio_net_supported_offloads(VirtIONet *n) +{ +return virtio_net_offloads_by_features(n-vdev.guest_features); +} + static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features) { VirtIONet *n = to_virtio_net(vdev); @@ -371,12 +413,8 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features) virtio_net_set_mrg_rx_bufs(n, !!(features (1 VIRTIO_NET_F_MRG_RXBUF))); if (n-has_vnet_hdr) { -tap_set_offload(qemu_get_subqueue(n-nic, 0)-peer, -(features VIRTIO_NET_F_GUEST_CSUM) 1, -(features VIRTIO_NET_F_GUEST_TSO4) 1, -(features VIRTIO_NET_F_GUEST_TSO6) 1, -(features VIRTIO_NET_F_GUEST_ECN) 1, -(features VIRTIO_NET_F_GUEST_UFO) 1); +n-curr_offloads = virtio_net_offloads_by_features(features); +virtio_net_apply_offloads(n); } for (i = 0; i n-max_queues; i++) { @@ -422,6 +460,42 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd, return VIRTIO_NET_OK; } +static int virtio_net_handle_offloads(VirtIONet *n, uint8_t cmd, + struct iovec *iov, unsigned int iov_cnt) +{ +uint32_t offloads; +size_t s; + +if (!((1 VIRTIO_NET_F_CTRL_GUEST_OFFLOADS) n-vdev.guest_features)) { +return VIRTIO_NET_ERR; +} + +s = iov_to_buf(iov, iov_cnt, 0, offloads, sizeof(offloads)); +if (s != sizeof(offloads)) { +return VIRTIO_NET_ERR; +} + +if (cmd == VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET) { +uint32_t supported_offloads; + +if (!n-has_vnet_hdr) { +return VIRTIO_NET_ERR; +} + +supported_offloads = virtio_net_supported_offloads(n); +if (offloads ~supported_offloads) { +return VIRTIO_NET_ERR; +} + +n-curr_offloads = offloads; +virtio_net_apply_offloads(n); + +return VIRTIO_NET_OK; +} else { +return VIRTIO_NET_ERR; +} +} + static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, struct iovec *iov, unsigned int iov_cnt) { @@ -591,6 +665,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) status = virtio_net_handle_vlan_table(n, ctrl.cmd, iov, iov_cnt); } else if (ctrl.class == VIRTIO_NET_CTRL_MQ) {
[Qemu-devel] [PATCH V11 03/17] qemu-img: remove unused parameter in collect_image_info()
Parameter *fmt was not used, so remove it. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Reviewed-by: Eric Blake ebl...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com --- qemu-img.c |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 31627b0..937ec01 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1640,8 +1640,7 @@ static void dump_json_image_info(ImageInfo *info) static void collect_image_info(BlockDriverState *bs, ImageInfo *info, - const char *filename, - const char *fmt) + const char *filename) { uint64_t total_sectors; char backing_filename[1024]; @@ -1815,7 +1814,7 @@ static ImageInfoList *collect_image_info_list(const char *filename, } info = g_new0(ImageInfo, 1); -collect_image_info(bs, info, filename, fmt); +collect_image_info(bs, info, filename); collect_snapshots(bs, info); elem = g_new0(ImageInfoList, 1); -- 1.7.1
[Qemu-devel] [PATCH V11 01/17] block: move bdrv_snapshot_find() to block/snapshot.c
This patch adds block/snapshot.c and then moves the function there. It also fixes small code style errors reported by check script. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Reviewed-by: Eric Blake ebl...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com --- block/Makefile.objs |1 + block/snapshot.c | 48 ++ include/block/snapshot.h | 37 +++ savevm.c | 23 +- 4 files changed, 87 insertions(+), 22 deletions(-) create mode 100644 block/snapshot.c create mode 100644 include/block/snapshot.h diff --git a/block/Makefile.objs b/block/Makefile.objs index c067f38..60a4cd2 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -3,6 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-c block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o block-obj-y += qed-check.o block-obj-y += parallels.o blkdebug.o blkverify.o +block-obj-y += snapshot.o block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o block-obj-$(CONFIG_POSIX) += raw-posix.o block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o diff --git a/block/snapshot.c b/block/snapshot.c new file mode 100644 index 000..c47a899 --- /dev/null +++ b/block/snapshot.c @@ -0,0 +1,48 @@ +/* + * Block layer snapshot related functions + * + * Copyright (c) 2003-2008 Fabrice Bellard + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include block/snapshot.h + +int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, + const char *name) +{ +QEMUSnapshotInfo *sn_tab, *sn; +int nb_sns, i, ret; + +ret = -ENOENT; +nb_sns = bdrv_snapshot_list(bs, sn_tab); +if (nb_sns 0) { +return ret; +} +for (i = 0; i nb_sns; i++) { +sn = sn_tab[i]; +if (!strcmp(sn-id_str, name) || !strcmp(sn-name, name)) { +*sn_info = *sn; +ret = 0; +break; +} +} +g_free(sn_tab); +return ret; +} diff --git a/include/block/snapshot.h b/include/block/snapshot.h new file mode 100644 index 000..4ad070c --- /dev/null +++ b/include/block/snapshot.h @@ -0,0 +1,37 @@ +/* + * Block layer snapshot related functions + * + * Copyright (c) 2003-2008 Fabrice Bellard + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef SNAPSHOT_H +#define SNAPSHOT_H + +#include qemu-common.h +/* + * block.h is needed for QEMUSnapshotInfo, it can be removed when define is + * moved here. + */ +#include block.h + +int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, + const char *name); +#endif diff --git a/savevm.c b/savevm.c index 406caa9..88acc38 100644 --- a/savevm.c +++ b/savevm.c @@ -40,6 +40,7 @@ #include trace.h #include qemu/bitops.h #include qemu/iov.h +#include
[Qemu-devel] [PATCH V11 00/17] qmp/hmp interfaces for internal snapshot info
In the use of snapshot a way to retrieve related info at runtime is needed, so this serial of patches will merge some code for qemu and qemu-img, and add or enchance following interfaces for qemu: 1) qmp: query-block, show info for a block device include image info Example: - { execute: query-block } - { return:[ { io-status: ok, device:ide0-hd0, locked:false, removable:false, inserted:{ ro:false, drv:qcow2, encrypted:false, file:disks/test.img, backing_file_depth:1, bps:100, bps_rd:0, bps_wr:0, iops:100, iops_rd:0, iops_wr:0, image:{ filename:disks/test.img, format:qcow2, virtual-size:2048000, backing_file:base.img, full-backing-filename:disks/base.img, backing-filename-format:qcow2, snapshots:[ { id: 1, name: snapshot1, vm-state-size: 0, date-sec: 1200, date-nsec: 12, vm-clock-sec: 206, vm-clock-nsec: 30 } ], backing-image:{ filename:disks/base.img, format:qcow2, virtual-size:2048000 } } }, type:unknown }, { io-status: ok, device:ide1-cd0, locked:false, removable:true, type:unknown }, { device:floppy0, locked:false, removable:true, type:unknown }, { device:sd0, locked:false, removable:true, type:unknown } ] } 2) qmp: query-snapshots, show avaiable vm snapshot info which can be used in loadvm. Example: - { execute: query-snapshots } - { return:[ { id: 1, name: snapshot1, vm-state-size: 0, date-sec: 1200, date-nsec: 12, vm-clock-sec: 206, vm-clock-nsec: 30 } ] } 3) hmp: info block, show what got in qmp query-block in monitor These patches follows the rule that use qmp to retieve information, hmp layer just does a translation from qmp object it got. To make code graceful, snapshot and image info retrieving code in qemu and qemu-img are merged into block layer, and some function name was adjusted to make it tips better. For the part touch by the serial, it works as: qemu qemu-img dump_monitordump_stdout |--| | block/qapi.c v2: Rename and adjusted qmp interface according to comments from Eric. Spelling fix. Information retrieving function in block layer goes to seperated patch. Free qmp object after usage in hmp. Added counterpart in qmp-commands.hx. Better tips in qmp-schema.json. v3: Spelling fix in commit message, patch 03/11. Spelling fix in code, patch 06/11. Add comments that vm-state-size is in bytes, and change size of it in example to a reasonable number, patch 08/11. v4: 02/13: in bdrv_get_filename(), add const to parameter *bs. 03/13: new added, in which the function correct the behavior in info retrieving. 04/13: in bdrv_query_snapshot_infolist(), remove NULL check before call err_setg(), added TODO comments that let block layer function set error instead of this layer to tip better for errors, Split out patch about image info to patch 05/13. 05/13: new splitted, and it checks *bs by calling bdrv_can_read_snapshot() before collect internal snasphot info to avoid *err is set unexpectly now. 06/13: check if error happens after calling bdrv_query_image_info(). 08/13: rename info to image in DeviceImageInfo and make it optional, when device is not inserted it will be empty, added error handling code when met error in calling block layer API. 09/13: distinguish *id and *name in bdrv_find_snapshots(), caller can choose what to search with. id_wellformed() should be called in new snapshot creation interface above this function in the future. 10/13: now this interface have addtional parameter *device, which enable showing internal snapshots on a single device. Also use bdrv_can_read_snapshot() instead of bdrv_can_snapshot() now. 11/13: this function goes to hmp.c so hmp_handler_error is not exported any more, split out patch that switch snapshot info function to patch 12/13. 12/13: new splitted. 13/13: use qmp API instead of directly calling block layer API, now all hmp function have correspond qmp funtion
[Qemu-devel] [PATCH V11 02/17] block: distinguish id and name in bdrv_find_snapshot()
To make it clear about id and name in searching, the API is changed a bit to distinguish them, and caller can choose to search by id or name. Searching will be done with higher priority of id. This function also returns negative value from bdrv_snapshot_list() instead of -ENOENT on error now. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Reviewed-by: Eric Blake ebl...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com --- block/snapshot.c | 46 ++ include/block/snapshot.h |2 +- savevm.c | 10 +- 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/block/snapshot.c b/block/snapshot.c index c47a899..7b2026c 100644 --- a/block/snapshot.c +++ b/block/snapshot.c @@ -24,8 +24,20 @@ #include block/snapshot.h +/* + * Try find an internal snapshot with @id or @name, @id have higher priority + * in searching. + * + * @bs: block device to search on, must not be NULL. + * @sn_info: snapshot information to be filled in, must not be NULL. + * @id: snapshot id to search with, can be NULL. + * @name: snapshot name to search with, can be NULL. + * + * returns 0 and @sn_info is filled with related information if found, + * otherwise it returns negative value. + */ int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, - const char *name) + const char *id, const char *name) { QEMUSnapshotInfo *sn_tab, *sn; int nb_sns, i, ret; @@ -33,16 +45,34 @@ int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, ret = -ENOENT; nb_sns = bdrv_snapshot_list(bs, sn_tab); if (nb_sns 0) { -return ret; +return nb_sns; } -for (i = 0; i nb_sns; i++) { -sn = sn_tab[i]; -if (!strcmp(sn-id_str, name) || !strcmp(sn-name, name)) { -*sn_info = *sn; -ret = 0; -break; + +/* search by id */ +if (id) { +for (i = 0; i nb_sns; i++) { +sn = sn_tab[i]; +if (!strcmp(sn-id_str, id)) { +*sn_info = *sn; +ret = 0; +goto out; +} } } + +/* search by name */ +if (name) { +for (i = 0; i nb_sns; i++) { +sn = sn_tab[i]; +if (!strcmp(sn-name, name)) { +*sn_info = *sn; +ret = 0; +goto out; +} +} +} + + out: g_free(sn_tab); return ret; } diff --git a/include/block/snapshot.h b/include/block/snapshot.h index 4ad070c..a047a8e 100644 --- a/include/block/snapshot.h +++ b/include/block/snapshot.h @@ -33,5 +33,5 @@ #include block.h int bdrv_snapshot_find(BlockDriverState *bs, QEMUSnapshotInfo *sn_info, - const char *name); + const char *id, const char *name); #endif diff --git a/savevm.c b/savevm.c index 88acc38..1d2da99 100644 --- a/savevm.c +++ b/savevm.c @@ -2212,7 +2212,7 @@ static int del_existing_snapshots(Monitor *mon, const char *name) bs = NULL; while ((bs = bdrv_next(bs))) { if (bdrv_can_snapshot(bs) -bdrv_snapshot_find(bs, snapshot, name) = 0) +bdrv_snapshot_find(bs, snapshot, name, name) = 0) { ret = bdrv_snapshot_delete(bs, name); if (ret 0) { @@ -2272,7 +2272,7 @@ void do_savevm(Monitor *mon, const QDict *qdict) sn-vm_clock_nsec = qemu_get_clock_ns(vm_clock); if (name) { -ret = bdrv_snapshot_find(bs, old_sn, name); +ret = bdrv_snapshot_find(bs, old_sn, name, name); if (ret = 0) { pstrcpy(sn-name, sizeof(sn-name), old_sn-name); pstrcpy(sn-id_str, sizeof(sn-id_str), old_sn-id_str); @@ -2363,7 +2363,7 @@ int load_vmstate(const char *name) } /* Don't even try to load empty VM states */ -ret = bdrv_snapshot_find(bs_vm_state, sn, name); +ret = bdrv_snapshot_find(bs_vm_state, sn, name, name); if (ret 0) { return ret; } else if (sn.vm_state_size == 0) { @@ -2387,7 +2387,7 @@ int load_vmstate(const char *name) return -ENOTSUP; } -ret = bdrv_snapshot_find(bs, sn, name); +ret = bdrv_snapshot_find(bs, sn, name, name); if (ret 0) { error_report(Device '%s' does not have the requested snapshot '%s', bdrv_get_device_name(bs), name); @@ -2493,7 +2493,7 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict) while ((bs1 = bdrv_next(bs1))) { if (bdrv_can_snapshot(bs1) bs1 != bs) { -ret = bdrv_snapshot_find(bs1, sn_info, sn-id_str); +ret = bdrv_snapshot_find(bs1, sn_info, sn-id_str, NULL); if (ret 0) { available = 0; break; -- 1.7.1
[Qemu-devel] [PATCH V11 04/17] block: move collect_snapshots() and collect_image_info() to block/qapi.c
This patch adds block/qapi.c and moves the functions there. To avoid conflict and tip better, macro in header file is BLOCK_QAPI_H instead of QAPI_H. The moving is for making review easier, those functions will be modified and renamed later. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Reviewed-by: Eric Blake ebl...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com --- block/Makefile.objs |2 +- block/qapi.c | 107 ++ include/block/qapi.h | 35 qemu-img.c | 86 +-- 4 files changed, 146 insertions(+), 84 deletions(-) create mode 100644 block/qapi.c create mode 100644 include/block/qapi.h diff --git a/block/Makefile.objs b/block/Makefile.objs index 60a4cd2..fc27bed 100644 --- a/block/Makefile.objs +++ b/block/Makefile.objs @@ -3,7 +3,7 @@ block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-c block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o block-obj-y += qed-check.o block-obj-y += parallels.o blkdebug.o blkverify.o -block-obj-y += snapshot.o +block-obj-y += snapshot.o qapi.o block-obj-$(CONFIG_WIN32) += raw-win32.o win32-aio.o block-obj-$(CONFIG_POSIX) += raw-posix.o block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o diff --git a/block/qapi.c b/block/qapi.c new file mode 100644 index 000..e2b1b6b --- /dev/null +++ b/block/qapi.c @@ -0,0 +1,107 @@ +/* + * Block layer qmp related functions + * + * Copyright (c) 2003-2008 Fabrice Bellard + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include block/qapi.h +#include block/block_int.h + +void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info) +{ +int i, sn_count; +QEMUSnapshotInfo *sn_tab = NULL; +SnapshotInfoList *info_list, *cur_item = NULL; +sn_count = bdrv_snapshot_list(bs, sn_tab); + +for (i = 0; i sn_count; i++) { +info-has_snapshots = true; +info_list = g_new0(SnapshotInfoList, 1); + +info_list-value= g_new0(SnapshotInfo, 1); +info_list-value-id= g_strdup(sn_tab[i].id_str); +info_list-value-name = g_strdup(sn_tab[i].name); +info_list-value-vm_state_size = sn_tab[i].vm_state_size; +info_list-value-date_sec = sn_tab[i].date_sec; +info_list-value-date_nsec = sn_tab[i].date_nsec; +info_list-value-vm_clock_sec = sn_tab[i].vm_clock_nsec / 10; +info_list-value-vm_clock_nsec = sn_tab[i].vm_clock_nsec % 10; + +/* XXX: waiting for the qapi to support qemu-queue.h types */ +if (!cur_item) { +info-snapshots = cur_item = info_list; +} else { +cur_item-next = info_list; +cur_item = info_list; +} + +} + +g_free(sn_tab); +} + +void bdrv_collect_image_info(BlockDriverState *bs, + ImageInfo *info, + const char *filename) +{ +uint64_t total_sectors; +char backing_filename[1024]; +char backing_filename2[1024]; +BlockDriverInfo bdi; + +bdrv_get_geometry(bs, total_sectors); + +info-filename= g_strdup(filename); +info-format = g_strdup(bdrv_get_format_name(bs)); +info-virtual_size= total_sectors * 512; +info-actual_size = bdrv_get_allocated_file_size(bs); +info-has_actual_size = info-actual_size = 0; +if (bdrv_is_encrypted(bs)) { +info-encrypted = true; +info-has_encrypted = true; +} +if (bdrv_get_info(bs, bdi) = 0) { +if (bdi.cluster_size != 0) { +info-cluster_size = bdi.cluster_size; +info-has_cluster_size = true; +} +info-dirty_flag = bdi.is_dirty; +info-has_dirty_flag = true; +} +bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename)); +if (backing_filename[0] !=
[Qemu-devel] [PATCH V11 05/17] block: add snapshot info query function bdrv_query_snapshot_info_list()
This patch adds function bdrv_query_snapshot_info_list(), which will retrieve snapshot info of an image in qmp object format. The implementation is based on the code moved from qemu-img.c with modification to fit more for qmp based block layer API. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- block/qapi.c | 55 ++--- include/block/qapi.h |4 ++- qemu-img.c |5 +++- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index e2b1b6b..03369a5 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -25,29 +25,56 @@ #include block/qapi.h #include block/block_int.h -void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info) +/* + * return 0 on success, @p_list will be set only on success, and caller need to + * check *p_list on success. + */ +int bdrv_query_snapshot_info_list(BlockDriverState *bs, + SnapshotInfoList **p_list, + Error **errp) { int i, sn_count; QEMUSnapshotInfo *sn_tab = NULL; -SnapshotInfoList *info_list, *cur_item = NULL; +SnapshotInfoList *info_list, *cur_item = NULL, *head = NULL; +SnapshotInfo *info; + sn_count = bdrv_snapshot_list(bs, sn_tab); +if (sn_count 0) { +const char *dev = bdrv_get_device_name(bs); +switch (sn_count) { +case -ENOMEDIUM: +error_setg(errp, Device '%s' is not inserted, dev); +break; +case -ENOTSUP: +error_setg(errp, + Device '%s' does not support internal snapshots, + dev); +break; +default: +error_setg_errno(errp, -sn_count, + Can't list snapshots of device '%s', dev); +break; +} +return sn_count; +} for (i = 0; i sn_count; i++) { -info-has_snapshots = true; -info_list = g_new0(SnapshotInfoList, 1); -info_list-value= g_new0(SnapshotInfo, 1); -info_list-value-id= g_strdup(sn_tab[i].id_str); -info_list-value-name = g_strdup(sn_tab[i].name); -info_list-value-vm_state_size = sn_tab[i].vm_state_size; -info_list-value-date_sec = sn_tab[i].date_sec; -info_list-value-date_nsec = sn_tab[i].date_nsec; -info_list-value-vm_clock_sec = sn_tab[i].vm_clock_nsec / 10; -info_list-value-vm_clock_nsec = sn_tab[i].vm_clock_nsec % 10; +info = g_new0(SnapshotInfo, 1); +info-id= g_strdup(sn_tab[i].id_str); +info-name = g_strdup(sn_tab[i].name); +info-vm_state_size = sn_tab[i].vm_state_size; +info-date_sec = sn_tab[i].date_sec; +info-date_nsec = sn_tab[i].date_nsec; +info-vm_clock_sec = sn_tab[i].vm_clock_nsec / 10; +info-vm_clock_nsec = sn_tab[i].vm_clock_nsec % 10; + +info_list = g_new0(SnapshotInfoList, 1); +info_list-value = info; /* XXX: waiting for the qapi to support qemu-queue.h types */ if (!cur_item) { -info-snapshots = cur_item = info_list; +head = cur_item = info_list; } else { cur_item-next = info_list; cur_item = info_list; @@ -56,6 +83,8 @@ void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info) } g_free(sn_tab); +*p_list = head; +return 0; } void bdrv_collect_image_info(BlockDriverState *bs, diff --git a/include/block/qapi.h b/include/block/qapi.h index 4586578..91dc41b 100644 --- a/include/block/qapi.h +++ b/include/block/qapi.h @@ -28,7 +28,9 @@ #include qapi-types.h #include block/block.h -void bdrv_collect_snapshots(BlockDriverState *bs , ImageInfo *info); +int bdrv_query_snapshot_info_list(BlockDriverState *bs, + SnapshotInfoList **p_list, + Error **errp); void bdrv_collect_image_info(BlockDriverState *bs, ImageInfo *info, const char *filename); diff --git a/qemu-img.c b/qemu-img.c index a020ccc..51043ef 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1735,7 +1735,10 @@ static ImageInfoList *collect_image_info_list(const char *filename, info = g_new0(ImageInfo, 1); bdrv_collect_image_info(bs, info, filename); -bdrv_collect_snapshots(bs, info); +if (!bdrv_query_snapshot_info_list(bs, info-snapshots, NULL) +info-snapshots) { +info-has_snapshots = true; +} elem = g_new0(ImageInfoList, 1); elem-value = info; -- 1.7.1
[Qemu-devel] [PATCH V11 07/17] block: add image info query function bdrv_query_image_info()
This patch adds function bdrv_query_image_info(), which will retrieve image info in qmp object format. The implementation is based on the code moved from qemu-img.c, but uses block layer function to get snapshot info. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- block/qapi.c | 41 ++--- include/block/qapi.h |6 +++--- qemu-img.c |8 ++-- 3 files changed, 39 insertions(+), 16 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index 19d4d93..176a479 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -122,18 +122,22 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs, return 0; } -void bdrv_collect_image_info(BlockDriverState *bs, - ImageInfo *info, - const char *filename) +/* return 0 on success, and @p_info will be set only on success. */ +int bdrv_query_image_info(BlockDriverState *bs, + ImageInfo **p_info, + Error **errp) { uint64_t total_sectors; -char backing_filename[1024]; +const char *backing_filename; char backing_filename2[1024]; BlockDriverInfo bdi; +int ret; +Error *err = NULL; +ImageInfo *info = g_new0(ImageInfo, 1); bdrv_get_geometry(bs, total_sectors); -info-filename= g_strdup(filename); +info-filename= g_strdup(bs-filename); info-format = g_strdup(bdrv_get_format_name(bs)); info-virtual_size= total_sectors * 512; info-actual_size = bdrv_get_allocated_file_size(bs); @@ -150,8 +154,8 @@ void bdrv_collect_image_info(BlockDriverState *bs, info-dirty_flag = bdi.is_dirty; info-has_dirty_flag = true; } -bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename)); -if (backing_filename[0] != '\0') { +backing_filename = bs-backing_file; +if (backing_filename backing_filename[0] != '\0') { info-backing_filename = g_strdup(backing_filename); info-has_backing_filename = true; bdrv_get_full_backing_filename(bs, backing_filename2, @@ -168,4 +172,27 @@ void bdrv_collect_image_info(BlockDriverState *bs, info-has_backing_filename_format = true; } } + +ret = bdrv_query_snapshot_info_list(bs, info-snapshots, false, err); +switch (ret) { +case 0: +if (info-snapshots) { +info-has_snapshots = true; +} +break; +/* recoverable error */ +case -ENOMEDIUM: +error_free(err); +break; +case -ENOTSUP: +error_free(err); +break; +default: +error_propagate(errp, err); +qapi_free_ImageInfo(info); +return ret; +} + +*p_info = info; +return 0; } diff --git a/include/block/qapi.h b/include/block/qapi.h index fe66053..2c62fdf 100644 --- a/include/block/qapi.h +++ b/include/block/qapi.h @@ -32,7 +32,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs, SnapshotInfoList **p_list, bool vm_snapshot, Error **errp); -void bdrv_collect_image_info(BlockDriverState *bs, - ImageInfo *info, - const char *filename); +int bdrv_query_image_info(BlockDriverState *bs, + ImageInfo **p_info, + Error **errp); #endif diff --git a/qemu-img.c b/qemu-img.c index 261c277..1dd0a60 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1733,12 +1733,8 @@ static ImageInfoList *collect_image_info_list(const char *filename, goto err; } -info = g_new0(ImageInfo, 1); -bdrv_collect_image_info(bs, info, filename); -if (!bdrv_query_snapshot_info_list(bs, info-snapshots, - false, NULL) -info-snapshots) { -info-has_snapshots = true; +if (bdrv_query_image_info(bs, info, NULL)) { +goto err; } elem = g_new0(ImageInfoList, 1); -- 1.7.1
[Qemu-devel] [PATCH V11 08/17] block: move qmp_query_block() and bdrv_query_info() to block/qapi.c
This is a code move patch, except in qmp_query_block bdrv_next(bs) is used instead of direct traverse of global array 'bdrv_states'. This patch also fix code style error reported by check script. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Reviewed-by: Kevin Wolf kw...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- block.c | 76 block/qapi.c | 77 + include/block/block.h |1 - include/block/qapi.h |1 + 4 files changed, 78 insertions(+), 77 deletions(-) diff --git a/block.c b/block.c index 0ae2e93..288cacd 100644 --- a/block.c +++ b/block.c @@ -3019,82 +3019,6 @@ int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base, return data.ret; } -BlockInfo *bdrv_query_info(BlockDriverState *bs) -{ -BlockInfo *info = g_malloc0(sizeof(*info)); -info-device = g_strdup(bs-device_name); -info-type = g_strdup(unknown); -info-locked = bdrv_dev_is_medium_locked(bs); -info-removable = bdrv_dev_has_removable_media(bs); - -if (bdrv_dev_has_removable_media(bs)) { -info-has_tray_open = true; -info-tray_open = bdrv_dev_is_tray_open(bs); -} - -if (bdrv_iostatus_is_enabled(bs)) { -info-has_io_status = true; -info-io_status = bs-iostatus; -} - -if (bs-dirty_bitmap) { -info-has_dirty = true; -info-dirty = g_malloc0(sizeof(*info-dirty)); -info-dirty-count = bdrv_get_dirty_count(bs) * BDRV_SECTOR_SIZE; -info-dirty-granularity = -((int64_t) BDRV_SECTOR_SIZE hbitmap_granularity(bs-dirty_bitmap)); -} - -if (bs-drv) { -info-has_inserted = true; -info-inserted = g_malloc0(sizeof(*info-inserted)); -info-inserted-file = g_strdup(bs-filename); -info-inserted-ro = bs-read_only; -info-inserted-drv = g_strdup(bs-drv-format_name); -info-inserted-encrypted = bs-encrypted; -info-inserted-encryption_key_missing = bdrv_key_required(bs); - -if (bs-backing_file[0]) { -info-inserted-has_backing_file = true; -info-inserted-backing_file = g_strdup(bs-backing_file); -} - -info-inserted-backing_file_depth = bdrv_get_backing_file_depth(bs); - -if (bs-io_limits_enabled) { -info-inserted-bps = - bs-io_limits.bps[BLOCK_IO_LIMIT_TOTAL]; -info-inserted-bps_rd = - bs-io_limits.bps[BLOCK_IO_LIMIT_READ]; -info-inserted-bps_wr = - bs-io_limits.bps[BLOCK_IO_LIMIT_WRITE]; -info-inserted-iops = - bs-io_limits.iops[BLOCK_IO_LIMIT_TOTAL]; -info-inserted-iops_rd = - bs-io_limits.iops[BLOCK_IO_LIMIT_READ]; -info-inserted-iops_wr = - bs-io_limits.iops[BLOCK_IO_LIMIT_WRITE]; -} -} -return info; -} - -BlockInfoList *qmp_query_block(Error **errp) -{ -BlockInfoList *head = NULL, **p_next = head; -BlockDriverState *bs; - -QTAILQ_FOREACH(bs, bdrv_states, list) { -BlockInfoList *info = g_malloc0(sizeof(*info)); -info-value = bdrv_query_info(bs); - -*p_next = info; -p_next = info-next; -} - -return head; -} - BlockStats *bdrv_query_stats(const BlockDriverState *bs) { BlockStats *s; diff --git a/block/qapi.c b/block/qapi.c index 176a479..6e0c7c3 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -25,6 +25,7 @@ #include block/qapi.h #include block/snapshot.h #include block/block_int.h +#include qmp-commands.h /* * check whether the snapshot is valid for whole vm. @@ -196,3 +197,79 @@ int bdrv_query_image_info(BlockDriverState *bs, *p_info = info; return 0; } + +BlockInfo *bdrv_query_info(BlockDriverState *bs) +{ +BlockInfo *info = g_malloc0(sizeof(*info)); +info-device = g_strdup(bs-device_name); +info-type = g_strdup(unknown); +info-locked = bdrv_dev_is_medium_locked(bs); +info-removable = bdrv_dev_has_removable_media(bs); + +if (bdrv_dev_has_removable_media(bs)) { +info-has_tray_open = true; +info-tray_open = bdrv_dev_is_tray_open(bs); +} + +if (bdrv_iostatus_is_enabled(bs)) { +info-has_io_status = true; +info-io_status = bs-iostatus; +} + +if (bs-dirty_bitmap) { +info-has_dirty = true; +info-dirty = g_malloc0(sizeof(*info-dirty)); +info-dirty-count = bdrv_get_dirty_count(bs) * BDRV_SECTOR_SIZE; +info-dirty-granularity = + ((int64_t) BDRV_SECTOR_SIZE hbitmap_granularity(bs-dirty_bitmap)); +} + +if (bs-drv) { +info-has_inserted = true; +info-inserted = g_malloc0(sizeof(*info-inserted)); +info-inserted-file = g_strdup(bs-filename); +info-inserted-ro = bs-read_only; +info-inserted-drv =
[Qemu-devel] [PATCH V11 10/17] qmp: add recursive member in ImageInfo
New member *backing-image is added to reflect the backing chain status. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Reviewed-by: Kevin Wolf kw...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- block/qapi.c |6 +- qapi-schema.json |5 - 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index 5e91ab8..fa61c85 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -123,7 +123,11 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs, return 0; } -/* return 0 on success, and @p_info will be set only on success. */ +/* + * return 0 on success, and @p_info will be set only on success, + * (*pinfo)-has_backing_image will be false and (*pinfo)-backing_image will + * be NULL. + */ int bdrv_query_image_info(BlockDriverState *bs, ImageInfo **p_info, Error **errp) diff --git a/qapi-schema.json b/qapi-schema.json index 225afef..ad9dd82 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -233,6 +233,8 @@ # # @snapshots: #optional list of VM snapshots # +# @backing-image: #optional info of the backing image (since 1.5) +# # Since: 1.3 # ## @@ -242,7 +244,8 @@ '*actual-size': 'int', 'virtual-size': 'int', '*cluster-size': 'int', '*encrypted': 'bool', '*backing-filename': 'str', '*full-backing-filename': 'str', - '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'] } } + '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'], + '*backing-image': 'ImageInfo' } } ## # @ImageCheck: -- 1.7.1
[Qemu-devel] [PATCH V11 06/17] block: add check for VM snapshot in bdrv_query_snapshot_info_list()
This patch adds a parameter to tell whether return valid snapshots for whole VM only. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Reviewed-by: Eric Blake ebl...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com --- block/qapi.c | 39 +-- include/block/qapi.h |1 + qemu-img.c |3 ++- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index 03369a5..19d4d93 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -23,14 +23,47 @@ */ #include block/qapi.h +#include block/snapshot.h #include block/block_int.h /* + * check whether the snapshot is valid for whole vm. + * + * @sn: snapshot info to be checked. + * @bs: where @sn was found. + * + * return true if the snapshot is consistent for the VM. + */ +static bool snapshot_valid_for_vm(const QEMUSnapshotInfo *sn, + BlockDriverState *bs) +{ +BlockDriverState *bs1 = NULL; +QEMUSnapshotInfo s, *sn_info = s; +int ret; + +/* Check logic is connected with load_vmstate(): + Only check the devices that can snapshot, other devices that can't + take snapshot, for example, readonly ones, will be ignored in + load_vmstate(). */ +while ((bs1 = bdrv_next(bs1))) { +if (bs1 != bs bdrv_can_snapshot(bs1)) { +ret = bdrv_snapshot_find(bs1, sn_info, sn-id_str, NULL); +if (ret 0) { +return false; +} +} +} +return true; +} + +/* * return 0 on success, @p_list will be set only on success, and caller need to - * check *p_list on success. + * check *p_list on success. If @vm_snapshot is true, limit the results to + * snapshots valid for the whole VM. */ int bdrv_query_snapshot_info_list(BlockDriverState *bs, SnapshotInfoList **p_list, + bool vm_snapshot, Error **errp) { int i, sn_count; @@ -59,7 +92,9 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs, } for (i = 0; i sn_count; i++) { - +if (vm_snapshot !snapshot_valid_for_vm(sn_tab[i], bs)) { +continue; +} info = g_new0(SnapshotInfo, 1); info-id= g_strdup(sn_tab[i].id_str); info-name = g_strdup(sn_tab[i].name); diff --git a/include/block/qapi.h b/include/block/qapi.h index 91dc41b..fe66053 100644 --- a/include/block/qapi.h +++ b/include/block/qapi.h @@ -30,6 +30,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs, SnapshotInfoList **p_list, + bool vm_snapshot, Error **errp); void bdrv_collect_image_info(BlockDriverState *bs, ImageInfo *info, diff --git a/qemu-img.c b/qemu-img.c index 51043ef..261c277 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1735,7 +1735,8 @@ static ImageInfoList *collect_image_info_list(const char *filename, info = g_new0(ImageInfo, 1); bdrv_collect_image_info(bs, info, filename); -if (!bdrv_query_snapshot_info_list(bs, info-snapshots, NULL) +if (!bdrv_query_snapshot_info_list(bs, info-snapshots, + false, NULL) info-snapshots) { info-has_snapshots = true; } -- 1.7.1
[Qemu-devel] [PATCH V11 11/17] qmp: add ImageInfo in BlockDeviceInfo used by query-block
Now image info will be retrieved as an embbed json object inside BlockDeviceInfo, backing chain info and all related internal snapshot info can be got in the enhanced recursive structure of ImageInfo. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- block/qapi.c | 38 ++-- include/block/qapi.h |4 ++- qapi-schema.json |5 +++- qmp-commands.hx | 67 +- 4 files changed, 108 insertions(+), 6 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index fa61c85..e98a028 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -202,9 +202,14 @@ int bdrv_query_image_info(BlockDriverState *bs, return 0; } -BlockInfo *bdrv_query_info(BlockDriverState *bs) +/* @p_info will be set only on success. */ +void bdrv_query_info(BlockDriverState *bs, + BlockInfo **p_info, + Error **errp) { BlockInfo *info = g_malloc0(sizeof(*info)); +BlockDriverState *bs0; +ImageInfo **p_image_info; info-device = g_strdup(bs-device_name); info-type = g_strdup(unknown); info-locked = bdrv_dev_is_medium_locked(bs); @@ -258,8 +263,28 @@ BlockInfo *bdrv_query_info(BlockDriverState *bs) info-inserted-iops_wr = bs-io_limits.iops[BLOCK_IO_LIMIT_WRITE]; } + +bs0 = bs; +p_image_info = info-inserted-image; +while (1) { +if (bdrv_query_image_info(bs0, p_image_info, errp)) { +goto err; +} +if (bs0-drv bs0-backing_hd) { +bs0 = bs0-backing_hd; +(*p_image_info)-has_backing_image = true; +p_image_info = ((*p_image_info)-backing_image); +} else { +break; +} +} } -return info; + +*p_info = info; +return; + + err: +qapi_free_BlockInfo(info); } SnapshotInfoList *qmp_query_snapshots(Error **errp) @@ -286,11 +311,18 @@ BlockInfoList *qmp_query_block(Error **errp) while ((bs = bdrv_next(bs))) { BlockInfoList *info = g_malloc0(sizeof(*info)); -info-value = bdrv_query_info(bs); +bdrv_query_info(bs, info-value, errp); +if (error_is_set(errp)) { +goto err; +} *p_next = info; p_next = info-next; } return head; + + err: +qapi_free_BlockInfoList(head); +return NULL; } diff --git a/include/block/qapi.h b/include/block/qapi.h index 0039a70..e0fd0a5 100644 --- a/include/block/qapi.h +++ b/include/block/qapi.h @@ -35,5 +35,7 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs, int bdrv_query_image_info(BlockDriverState *bs, ImageInfo **p_info, Error **errp); -BlockInfo *bdrv_query_info(BlockDriverState *bs); +void bdrv_query_info(BlockDriverState *bs, + BlockInfo **p_info, + Error **errp); #endif diff --git a/qapi-schema.json b/qapi-schema.json index ad9dd82..02dabc3 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -756,6 +756,8 @@ # # @iops_wr: write I/O operations per second is specified # +# @image: the info of image used (since: 1.5) +# # Since: 0.14.0 # # Notes: This interface is only found in @BlockInfo. @@ -765,7 +767,8 @@ '*backing_file': 'str', 'backing_file_depth': 'int', 'encrypted': 'bool', 'encryption_key_missing': 'bool', 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int', -'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} } +'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int', +'image': 'ImageInfo' } } ## # @BlockDeviceIoStatus: diff --git a/qmp-commands.hx b/qmp-commands.hx index 6b20684..b856be7 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1703,6 +1703,47 @@ Each json-object contain the following: - iops: limit total I/O operations per second (json-int) - iops_rd: limit read operations per second (json-int) - iops_wr: limit write operations per second (json-int) + - image: the detail of the image, it is a json-object containing +the following: + - filename: image file name (json-string) + - format: image format (json-string) + - virtual-size: image capacity in bytes (json-int) + - dirty-flag: true if image is not cleanly closed, not present + means clean (json-bool, optional) + - actual-size: actual size on disk in bytes of the image, not + present when image does not support thin + provision (json-int, optional) + - cluster-size: size of a cluster in bytes, not present if image + format does not support it (json-int, optional) + - encrypted: true if the image is encrypted, not present means +
[Qemu-devel] [PATCH V11 12/17] block: move bdrv_snapshot_dump() and dump_human_image_info() to block/qapi.c
They are needed later in hmp command, dump_human_image_info() is renamed to bdrv_image_info_dump(). Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- block.c | 33 block/qapi.c | 100 + include/block/block.h |1 - include/block/qapi.h |2 + qemu-img.c| 69 +- savevm.c |1 + 6 files changed, 104 insertions(+), 102 deletions(-) diff --git a/block.c b/block.c index 288cacd..cd4bc7e 100644 --- a/block.c +++ b/block.c @@ -3433,39 +3433,6 @@ char *get_human_readable_size(char *buf, int buf_size, int64_t size) return buf; } -char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn) -{ -char buf1[128], date_buf[128], clock_buf[128]; -struct tm tm; -time_t ti; -int64_t secs; - -if (!sn) { -snprintf(buf, buf_size, - %-10s%-20s%7s%20s%15s, - ID, TAG, VM SIZE, DATE, VM CLOCK); -} else { -ti = sn-date_sec; -localtime_r(ti, tm); -strftime(date_buf, sizeof(date_buf), - %Y-%m-%d %H:%M:%S, tm); -secs = sn-vm_clock_nsec / 10; -snprintf(clock_buf, sizeof(clock_buf), - %02d:%02d:%02d.%03d, - (int)(secs / 3600), - (int)((secs / 60) % 60), - (int)(secs % 60), - (int)((sn-vm_clock_nsec / 100) % 1000)); -snprintf(buf, buf_size, - %-10s%-20s%7s%20s%15s, - sn-id_str, sn-name, - get_human_readable_size(buf1, sizeof(buf1), sn-vm_state_size), - date_buf, - clock_buf); -} -return buf; -} - /**/ /* async I/Os */ diff --git a/block/qapi.c b/block/qapi.c index e98a028..df63c97 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -326,3 +326,103 @@ BlockInfoList *qmp_query_block(Error **errp) qapi_free_BlockInfoList(head); return NULL; } + +char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn) +{ +char buf1[128], date_buf[128], clock_buf[128]; +struct tm tm; +time_t ti; +int64_t secs; + +if (!sn) { +snprintf(buf, buf_size, + %-10s%-20s%7s%20s%15s, + ID, TAG, VM SIZE, DATE, VM CLOCK); +} else { +ti = sn-date_sec; +localtime_r(ti, tm); +strftime(date_buf, sizeof(date_buf), + %Y-%m-%d %H:%M:%S, tm); +secs = sn-vm_clock_nsec / 10; +snprintf(clock_buf, sizeof(clock_buf), + %02d:%02d:%02d.%03d, + (int)(secs / 3600), + (int)((secs / 60) % 60), + (int)(secs % 60), + (int)((sn-vm_clock_nsec / 100) % 1000)); +snprintf(buf, buf_size, + %-10s%-20s%7s%20s%15s, + sn-id_str, sn-name, + get_human_readable_size(buf1, sizeof(buf1), sn-vm_state_size), + date_buf, + clock_buf); +} +return buf; +} + +void bdrv_image_info_dump(ImageInfo *info) +{ +char size_buf[128], dsize_buf[128]; +if (!info-has_actual_size) { +snprintf(dsize_buf, sizeof(dsize_buf), unavailable); +} else { +get_human_readable_size(dsize_buf, sizeof(dsize_buf), +info-actual_size); +} +get_human_readable_size(size_buf, sizeof(size_buf), info-virtual_size); +printf(image: %s\n + file format: %s\n + virtual size: %s (% PRId64 bytes)\n + disk size: %s\n, + info-filename, info-format, size_buf, + info-virtual_size, + dsize_buf); + +if (info-has_encrypted info-encrypted) { +printf(encrypted: yes\n); +} + +if (info-has_cluster_size) { +printf(cluster_size: % PRId64 \n, info-cluster_size); +} + +if (info-has_dirty_flag info-dirty_flag) { +printf(cleanly shut down: no\n); +} + +if (info-has_backing_filename) { +printf(backing file: %s, info-backing_filename); +if (info-has_full_backing_filename) { +printf( (actual path: %s), info-full_backing_filename); +} +putchar('\n'); +if (info-has_backing_filename_format) { +printf(backing file format: %s\n, info-backing_filename_format); +} +} + +if (info-has_snapshots) { +SnapshotInfoList *elem; +char buf[256]; + +printf(Snapshot list:\n); +printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), NULL)); + +/* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but + * we convert to the block layer's native QEMUSnapshotInfo for now. + */ +for (elem = info-snapshots; elem; elem = elem-next) { +QEMUSnapshotInfo sn = { +
[Qemu-devel] [PATCH V11 17/17] hmp: add parameters device and -v for info block
With these parameters, user can choose the information to be showed, to avoid message flood in the montior. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- hmp.c | 34 -- monitor.c |7 --- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/hmp.c b/hmp.c index 6f93fcd..677bc7f 100644 --- a/hmp.c +++ b/hmp.c @@ -280,10 +280,15 @@ void hmp_info_block(Monitor *mon, const QDict *qdict) BlockInfoList *block_list, *info; ImageInfo *image_info; GString *buf = NULL; +const char *device = qdict_get_try_str(qdict, device); +int verbose = qdict_get_try_bool(qdict, verbose, 0); block_list = qmp_query_block(NULL); for (info = block_list; info; info = info-next) { +if (device strcmp(device, info-value-device)) { +continue; +} monitor_printf(mon, %s: removable=%d, info-value-device, info-value-removable); @@ -322,22 +327,23 @@ void hmp_info_block(Monitor *mon, const QDict *qdict) info-value-inserted-iops_rd, info-value-inserted-iops_wr); -if (!buf) { -buf = g_string_sized_new(2048); -} -monitor_printf(mon, images:\n); -image_info = info-value-inserted-image; -while (1) { -bdrv_image_info_dump(buf, image_info); -monitor_printf(mon, %s, buf-str); -g_string_set_size(buf, 0); -if (image_info-has_backing_image) { -image_info = image_info-backing_image; -} else { -break; +if (verbose) { +if (!buf) { +buf = g_string_sized_new(2048); +} +monitor_printf(mon, images:\n); +image_info = info-value-inserted-image; +while (1) { +bdrv_image_info_dump(buf, image_info); +monitor_printf(mon, %s, buf-str); +g_string_set_size(buf, 0); +if (image_info-has_backing_image) { +image_info = image_info-backing_image; +} else { +break; +} } } - } else { monitor_printf(mon, [not inserted]); } diff --git a/monitor.c b/monitor.c index 4720207..ae5a61e 100644 --- a/monitor.c +++ b/monitor.c @@ -2465,9 +2465,10 @@ static mon_cmd_t info_cmds[] = { }, { .name = block, -.args_type = , -.params = , -.help = show the block devices, +.args_type = verbose:-v,device:B?, +.params = [-v] [device], +.help = show info of one block device or all block devices + (and details of images with -v option), .mhandler.cmd = hmp_info_block, }, { -- 1.7.1
[Qemu-devel] [PATCH V11 14/17] hmp: add function hmp_info_snapshots()
This function will simply call qmp interface qmp_query_snapshots() added in last commit and then dump information in monitor console. To get snapshot info, Now qemu and qemu-img both call block layer function bdrv_query_snapshot_info_list() in their calling path, and then they just translate the qmp object got to strings in stdout or monitor console. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- hmp.c | 48 hmp.h |1 + 2 files changed, 49 insertions(+), 0 deletions(-) diff --git a/hmp.c b/hmp.c index dbe9b90..89e1aaf 100644 --- a/hmp.c +++ b/hmp.c @@ -22,6 +22,7 @@ #include qemu/sockets.h #include monitor/monitor.h #include ui/console.h +#include block/qapi.h static void hmp_handle_error(Monitor *mon, Error **errp) { @@ -653,6 +654,53 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict) qapi_free_TPMInfoList(info_list); } +/* assume list is valid */ +static void monitor_dump_snapshotinfolist(Monitor *mon, SnapshotInfoList *list) +{ +SnapshotInfoList *elem; +GString *buf = g_string_sized_new(1024); + +bdrv_snapshot_dump(buf, NULL); +g_string_append_printf(buf, \n); + +for (elem = list; elem; elem = elem-next) { +QEMUSnapshotInfo sn = { +.vm_state_size = elem-value-vm_state_size, +.date_sec = elem-value-date_sec, +.date_nsec = elem-value-date_nsec, +.vm_clock_nsec = elem-value-vm_clock_sec * 10ULL + + elem-value-vm_clock_nsec, +}; +pstrcpy(sn.id_str, sizeof(sn.id_str), elem-value-id); +pstrcpy(sn.name, sizeof(sn.name), elem-value-name); +bdrv_snapshot_dump(buf, sn); +g_string_append_printf(buf, \n); +} + +monitor_printf(mon, %s, buf-str); +g_string_free(buf, true); +} + +void hmp_info_snapshots(Monitor *mon, const QDict *qdict) +{ +Error *err = NULL; +SnapshotInfoList *list; + +list = qmp_query_snapshots(err); +if (error_is_set(err)) { +hmp_handle_error(mon, err); +return; +} + +if (list == NULL) { +monitor_printf(mon, There is no suitable snapshot available\n); +return; +} + +monitor_dump_snapshotinfolist(mon, list); +qapi_free_SnapshotInfoList(list); +} + void hmp_quit(Monitor *mon, const QDict *qdict) { monitor_suspend(mon); diff --git a/hmp.h b/hmp.h index 80e8b41..1172c47 100644 --- a/hmp.h +++ b/hmp.h @@ -37,6 +37,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict); void hmp_info_pci(Monitor *mon, const QDict *qdict); void hmp_info_block_jobs(Monitor *mon, const QDict *qdict); void hmp_info_tpm(Monitor *mon, const QDict *qdict); +void hmp_info_snapshots(Monitor *mon, const QDict *qdict); void hmp_quit(Monitor *mon, const QDict *qdict); void hmp_stop(Monitor *mon, const QDict *qdict); void hmp_system_reset(Monitor *mon, const QDict *qdict); -- 1.7.1
[Qemu-devel] [PATCH V11 13/17] block: dump to buffer for bdrv_snapshot_dump() and bdrv_image_info_dump()
This patch would allow hmp use them just like qemu-img, and avoid string truncation. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- block/qapi.c | 65 +++--- include/block/qapi.h |4 +- qemu-img.c | 22 savevm.c | 11 ++-- 4 files changed, 61 insertions(+), 41 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index df63c97..e27519e 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -327,7 +327,7 @@ BlockInfoList *qmp_query_block(Error **errp) return NULL; } -char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn) +void bdrv_snapshot_dump(GString *buf, QEMUSnapshotInfo *sn) { char buf1[128], date_buf[128], clock_buf[128]; struct tm tm; @@ -335,9 +335,8 @@ char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn) int64_t secs; if (!sn) { -snprintf(buf, buf_size, - %-10s%-20s%7s%20s%15s, - ID, TAG, VM SIZE, DATE, VM CLOCK); +g_string_append_printf(buf, %-10s%-20s%7s%20s%15s, + ID, TAG, VM SIZE, DATE, VM CLOCK); } else { ti = sn-date_sec; localtime_r(ti, tm); @@ -350,17 +349,17 @@ char *bdrv_snapshot_dump(char *buf, int buf_size, QEMUSnapshotInfo *sn) (int)((secs / 60) % 60), (int)(secs % 60), (int)((sn-vm_clock_nsec / 100) % 1000)); -snprintf(buf, buf_size, - %-10s%-20s%7s%20s%15s, - sn-id_str, sn-name, - get_human_readable_size(buf1, sizeof(buf1), sn-vm_state_size), - date_buf, - clock_buf); +g_string_append_printf(buf, + %-10s%-20s%7s%20s%15s, + sn-id_str, sn-name, + get_human_readable_size(buf1, sizeof(buf1), + sn-vm_state_size), + date_buf, + clock_buf); } -return buf; } -void bdrv_image_info_dump(ImageInfo *info) +void bdrv_image_info_dump(GString *buf, ImageInfo *info) { char size_buf[128], dsize_buf[128]; if (!info-has_actual_size) { @@ -370,43 +369,48 @@ void bdrv_image_info_dump(ImageInfo *info) info-actual_size); } get_human_readable_size(size_buf, sizeof(size_buf), info-virtual_size); -printf(image: %s\n - file format: %s\n - virtual size: %s (% PRId64 bytes)\n - disk size: %s\n, - info-filename, info-format, size_buf, - info-virtual_size, - dsize_buf); +g_string_append_printf(buf, + image: %s\n + file format: %s\n + virtual size: %s (% PRId64 bytes)\n + disk size: %s\n, + info-filename, info-format, size_buf, + info-virtual_size, + dsize_buf); if (info-has_encrypted info-encrypted) { -printf(encrypted: yes\n); +g_string_append_printf(buf, encrypted: yes\n); } if (info-has_cluster_size) { -printf(cluster_size: % PRId64 \n, info-cluster_size); +g_string_append_printf(buf, cluster_size: % PRId64 \n, + info-cluster_size); } if (info-has_dirty_flag info-dirty_flag) { -printf(cleanly shut down: no\n); +g_string_append_printf(buf, cleanly shut down: no\n); } if (info-has_backing_filename) { -printf(backing file: %s, info-backing_filename); +g_string_append_printf(buf, backing file: %s, + info-backing_filename); if (info-has_full_backing_filename) { -printf( (actual path: %s), info-full_backing_filename); +g_string_append_printf(buf, (actual path: %s), + info-full_backing_filename); } -putchar('\n'); +g_string_append_printf(buf, \n); if (info-has_backing_filename_format) { -printf(backing file format: %s\n, info-backing_filename_format); +g_string_append_printf(buf, backing file format: %s\n, + info-backing_filename_format); } } if (info-has_snapshots) { SnapshotInfoList *elem; -char buf[256]; -printf(Snapshot list:\n); -printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), NULL)); +g_string_append_printf(buf, Snapshot list:\n); +bdrv_snapshot_dump(buf, NULL); +g_string_append_printf(buf, \n); /* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but * we convert to the block layer's native QEMUSnapshotInfo for now. @@ -422,7
[Qemu-devel] [PATCH V11 15/17] hmp: switch snapshot info function to qmp based one
This patch using new added function in last commit which retrieve info from qmp for snapshot info. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Reviewed-by: Kevin Wolf kw...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- monitor.c |2 +- savevm.c | 69 - 2 files changed, 1 insertions(+), 70 deletions(-) diff --git a/monitor.c b/monitor.c index 4ec1db9..4720207 100644 --- a/monitor.c +++ b/monitor.c @@ -2621,7 +2621,7 @@ static mon_cmd_t info_cmds[] = { .args_type = , .params = , .help = show the currently saved VM snapshots, -.mhandler.cmd = do_info_snapshots, +.mhandler.cmd = hmp_info_snapshots, }, { .name = status, diff --git a/savevm.c b/savevm.c index ce0bbe1..ca1e393 100644 --- a/savevm.c +++ b/savevm.c @@ -2459,75 +2459,6 @@ void do_delvm(Monitor *mon, const QDict *qdict) } } -void do_info_snapshots(Monitor *mon, const QDict *qdict) -{ -BlockDriverState *bs, *bs1; -QEMUSnapshotInfo *sn_tab, *sn, s, *sn_info = s; -int nb_sns, i, ret, available; -int total; -int *available_snapshots; -GString *buf = NULL; - -bs = bdrv_snapshots(); -if (!bs) { -monitor_printf(mon, No available block device supports snapshots\n); -return; -} - -nb_sns = bdrv_snapshot_list(bs, sn_tab); -if (nb_sns 0) { -monitor_printf(mon, bdrv_snapshot_list: error %d\n, nb_sns); -return; -} - -if (nb_sns == 0) { -monitor_printf(mon, There is no snapshot available.\n); -return; -} - -available_snapshots = g_malloc0(sizeof(int) * nb_sns); -total = 0; -for (i = 0; i nb_sns; i++) { -sn = sn_tab[i]; -available = 1; -bs1 = NULL; - -while ((bs1 = bdrv_next(bs1))) { -if (bdrv_can_snapshot(bs1) bs1 != bs) { -ret = bdrv_snapshot_find(bs1, sn_info, sn-id_str, NULL); -if (ret 0) { -available = 0; -break; -} -} -} - -if (available) { -available_snapshots[total] = i; -total++; -} -} - -if (total 0) { -buf = g_string_new(NULL); -bdrv_snapshot_dump(buf, NULL); -g_string_append_printf(buf, \n); -for (i = 0; i total; i++) { -sn = sn_tab[available_snapshots[i]]; -bdrv_snapshot_dump(buf, sn); -g_string_append_printf(buf, \n); -} -monitor_printf(mon, %s, buf-str); -g_string_free(buf, true); -} else { -monitor_printf(mon, There is no suitable snapshot available\n); -} - -g_free(sn_tab); -g_free(available_snapshots); - -} - void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev) { qemu_ram_set_idstr(memory_region_get_ram_addr(mr) TARGET_PAGE_MASK, -- 1.7.1
[Qemu-devel] [PATCH V11 16/17] hmp: show ImageInfo in 'info block'
Now human monitor can show image details include internal snapshot info for every block device. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com --- hmp.c | 22 ++ 1 files changed, 22 insertions(+), 0 deletions(-) diff --git a/hmp.c b/hmp.c index 89e1aaf..6f93fcd 100644 --- a/hmp.c +++ b/hmp.c @@ -278,6 +278,8 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict) void hmp_info_block(Monitor *mon, const QDict *qdict) { BlockInfoList *block_list, *info; +ImageInfo *image_info; +GString *buf = NULL; block_list = qmp_query_block(NULL); @@ -319,6 +321,23 @@ void hmp_info_block(Monitor *mon, const QDict *qdict) info-value-inserted-iops, info-value-inserted-iops_rd, info-value-inserted-iops_wr); + +if (!buf) { +buf = g_string_sized_new(2048); +} +monitor_printf(mon, images:\n); +image_info = info-value-inserted-image; +while (1) { +bdrv_image_info_dump(buf, image_info); +monitor_printf(mon, %s, buf-str); +g_string_set_size(buf, 0); +if (image_info-has_backing_image) { +image_info = image_info-backing_image; +} else { +break; +} +} + } else { monitor_printf(mon, [not inserted]); } @@ -326,6 +345,9 @@ void hmp_info_block(Monitor *mon, const QDict *qdict) monitor_printf(mon, \n); } +if (buf) { +g_string_free(buf, true); +} qapi_free_BlockInfoList(block_list); } -- 1.7.1
Re: [Qemu-devel] [PATCH 0/2 V3] virtio-spec/net: dynamic network offloads configuration
On Tue, Apr 02, 2013 at 02:38:58PM +0300, Dmitry Fleytman wrote: From: Dmitry Fleytman dfley...@redhat.com V3 changes: 1. Compat macro added 2. Feature name beautification V2 changes: 1. _GUEST_ added to command and feature names 2. Live migration logic fixed Reported-by: Michael S. Tsirkin m...@redhat.com Acked-by: Michael S. Tsirkin m...@redhat.com Rusty? One of recently introduced Windows features (RSC) requires network driver to be able to enable and disable HW LRO offload on the fly without device reinitialization. Current Virtio specification doesn't support this requirement. The solution proposed by following spec patch is to add a new control command for this purpose. The same solution may be used in Linux driver for ethtool interface implementation. Patch for QEMU mainline that implements this specification change attached as well. -- 1.8.1.4
Re: [Qemu-devel] [PATCH] vga.c: Debug messages go to stderr
On 04/02/13 12:53, Alex DAMIAN wrote: From: Alexandru DAMIAN alexandru.dam...@intel.com Move debug messages to stderr since often in vga code debug stdio is used as serial console. Make them tracepoints would be even better ... cheers, Gerd
[Qemu-devel] [PATCH V11 09/17] qmp: add interface query-snapshots
This interface returns info of valid internal snapshots for whole vm. Signed-off-by: Wenchao Xia xiaw...@linux.vnet.ibm.com Reviewed-by: Eric Blake ebl...@redhat.com Reviewed-by: Kevin Wolf kw...@redhat.com --- block/qapi.c | 17 qapi-schema.json | 14 + qmp-commands.hx | 55 ++ 3 files changed, 86 insertions(+), 0 deletions(-) diff --git a/block/qapi.c b/block/qapi.c index 6e0c7c3..5e91ab8 100644 --- a/block/qapi.c +++ b/block/qapi.c @@ -258,6 +258,23 @@ BlockInfo *bdrv_query_info(BlockDriverState *bs) return info; } +SnapshotInfoList *qmp_query_snapshots(Error **errp) +{ +BlockDriverState *bs; +SnapshotInfoList *list = NULL; + +/* internal snapshot for whole vm */ +bs = bdrv_snapshots(); +if (!bs) { +error_setg(errp, No available block device supports snapshots\n); +return NULL; +} + +bdrv_query_snapshot_info_list(bs, list, true, errp); + +return list; +} + BlockInfoList *qmp_query_block(Error **errp) { BlockInfoList *head = NULL, **p_next = head; diff --git a/qapi-schema.json b/qapi-schema.json index f629a24..225afef 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -841,6 +841,20 @@ { 'command': 'query-block', 'returns': ['BlockInfo'] } ## +# @query-snapshots: +# +# Get a list of internal snapshots for the whole virtual machine. Only valid +# internal snapshots will be returned, inconsistent ones will be ignored +# +# Returns: a list of @SnapshotInfo describing all consistent virtual machine +# snapshots +# +# Since: 1.5 +## +{ 'command': 'query-snapshots', + 'returns': ['SnapshotInfo'] } + +## # @BlockDeviceStats: # # Statistics of a virtual block device or a block backing device. diff --git a/qmp-commands.hx b/qmp-commands.hx index 1e0e11e..6b20684 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -1765,6 +1765,61 @@ EQMP }, SQMP +query-snapshots +--- + +Show the internal consistent snapshot information + +Each snapshot is represented by a json-object. The returned value +is a json-array of all snapshots + +Each json-object contain the following: + +- id: unique snapshot id (json-string) +- name: internal snapshot name (json-string) +- vm-state-size: size of the VM state in bytes (json-int) +- date-sec: UTC date of the snapshot in seconds (json-int) +- date-nsec: fractional part in nanoseconds to be used with + date-sec(json-int) +- vm-clock-sec: VM clock relative to boot in seconds (json-int) +- vm-clock-nsec: fractional part in nanoseconds to be used with + vm-clock-sec (json-int) + +Example: + +- { execute: query-snapshots } +- { + return:[ + { +id: 1, +name: snapshot1, +vm-state-size: 0, +date-sec: 1200, +date-nsec: 12, +vm-clock-sec: 206, +vm-clock-nsec: 30 + }, + { +id: 2, +name: snapshot2, +vm-state-size: 3400, +date-sec: 13000200, +date-nsec: 32, +vm-clock-sec: 406, +vm-clock-nsec: 31 + } + ] + } + +EQMP + +{ +.name = query-snapshots, +.args_type = , +.mhandler.cmd_new = qmp_marshal_input_query_snapshots, +}, + +SQMP query-blockstats -- 1.7.1
Re: [Qemu-devel] [PATCH-v2 0/3] virtio/vhost: Add checks for uninitialized VQs
On Mon, Apr 01, 2013 at 11:58:21PM +, Nicholas A. Bellinger wrote: From: Nicholas Bellinger n...@linux-iscsi.org Hi folks, This series adds a virtio_queue_valid() for use by virtio-pci code in order to prevent opreations upon uninitialized VQs, which is currently expected to occur during seabios setup of virtio-scsi with in-flight vhost-scsi-pci device code. On the vhost side, it also adds virtio_queue_valid() sanity checks in vhost_virtqueue_[start,stop]() and vhost_verify_ring_mappings() in order to skip the same uninitialized VQs. Changes from v1: - Remove now unnecessary virtio_queue_get_num() calls in virtio-pci.c - Add virtio_queue_valid() calls in vhost_virtqueue_[start,stop]() Please review. --nab Looks reasonable. Acked-by: Michael S. Tsirkin m...@redhat.com So - does this fix the issues you saw with vhost-scsi? Michael S. Tsirkin (1): virtio: add API to check that ring is setup Nicholas Bellinger (2): virtio-pci: Add virtio_queue_valid checks ahead of virtio_queue_get_num vhost: Skip uninitialized VQs in vhost_virtqueue_[start,stop] hw/vhost.c | 12 hw/virtio-pci.c | 34 +++--- hw/virtio.c |5 + hw/virtio.h |1 + 4 files changed, 33 insertions(+), 19 deletions(-) -- 1.7.2.5
Re: [Qemu-devel] [PATCH v4 0/7] virtio-serial refactoring.
On 29 March 2013 09:02, fred.kon...@greensocs.com wrote: From: KONRAD Frederic fred.kon...@greensocs.com This is the next part of virtio-refactoring. Basically it creates virtio-serial device which extends virtio-device. Then a virtio-serial can be connected on a virtio-bus. virtio-serial-pci, virtio-serial-s390 and virtio-serial-ccw are created too, they extend respectively virtio-pci, virtio-s390-device, virtio-ccw-device and have a virtio-serial. Series: Reviewed-by: Peter Maydell peter.mayd...@linaro.org -- PMM
[Qemu-devel] [Patch] v2 fix /proc/self/maps output
Add a space at end of line when there is no filename to print, to conform to linux kernel format (see show_map_vma() in fs/proc/task_mmu.c). Signed-off-by: Christophe Lyon christophe.l...@linaro.org --- Changes v1-v2: Additional space is now part of the format string. linux-user/syscall.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index a148d9f..c3c5c11 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -5013,10 +5013,10 @@ static int open_self_maps(void *cpu_env, int fd) } if (h2g_valid(min) h2g_valid(max)) { dprintf(fd, TARGET_ABI_FMT_lx - TARGET_ABI_FMT_lx - %c%c%c%c %08 PRIx64 %02x:%02x %d%s%s\n, + %c%c%c%c %08 PRIx64 %02x:%02x %d %s%s\n, h2g(min), h2g(max), flag_r, flag_w, flag_x, flag_p, offset, dev_maj, dev_min, inode, -path[0] ?: , path); +path[0] ? : , path); } } -- 1.7.10.4
Re: [Qemu-devel] vNVRAM / blobstore design
On Sun, Mar 31, 2013 at 04:48:24PM -0400, Kenneth Goldman wrote: Michael S. Tsirkin m...@redhat.com wrote on 03/31/2013 04:17:28 AM: You want to protect against someone who is able to manipulate some bits in the file (content) but not others (hash)? What's the attack you are trying to protect against here? I'm guessing the only result of extra checksums would be unbootable guests when qemu manages to corrupt the checksum without any help from attackers ... You are of course correct. I advised an integrity value just to detect a hardware or software fault. The check value would not protect against an attack. Fair enough, but why protect these bits specifically? E.g. disk corruption seems more likely (since it's bigger). Add integrity at that level? Why even stop at detection, let's do error correction ... -- MST
[Qemu-devel] [Bug 1042388] Re: qemu: Unsupported syscall: 257 (timer_create)
Any news on this? -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1042388 Title: qemu: Unsupported syscall: 257 (timer_create) Status in QEMU: Confirmed Bug description: Running qemu-arm-static for git HEAD. When I try to install ghc from debian into my arm chroot I get: Setting up ghc (7.4.1-4) ... qemu: Unsupported syscall: 257 ghc: timer_create: Function not implemented qemu: Unsupported syscall: 257 ghc-pkg: timer_create: Function not implemented dpkg: error processing ghc (--configure): subprocess installed post-installation script returned error exit status 1 Errors were encountered while processing: ghc E: Sub-process /usr/bin/dpkg returned an error code (1) To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1042388/+subscriptions
Re: [Qemu-devel] [PATCH 1/1] rng backend: open backend in blocking mode
Amit Shah amit.s...@redhat.com writes: On (Mon) 01 Apr 2013 [09:02:46], Anthony Liguori wrote: Amit Shah amit.s...@redhat.com writes: Opening backends in non-blocking mode isn't necessary, we don't do anything while waiting for data. This also excuses us from checking for EAGAIN, which for the default random backend, is a very common return error type. It's not common... It really shouldn't happen however. EAGAIN is common when a file is opened in non-blocking mode. Needs to be made verbose? EAGAIN doesn't just happen randomly. It only happens when you read from an fd when no data is present. Normally, that is something that is predictable. Starting the guest with '-device virtio-rng-pci', issuing a 'cat /dev/hwrng' in the guest while also doing 'cat /dev/random' on the host causes You are essentially cat'ing the same device twice. What's happening is that there is entropy available in /dev/random so a select() notification happens but before we are able to read() it, the cat of /dev/hwrng ends up consuming that entropy. This would never happen with a socket, for instance. /dev/random is special in there are multiple readers. backends/rng-random.c:44:entropy_available: assertion failed: (len != -1) without this fix. This fix would cause QEMU to block indefinitely which I don't think is very good behavior. I think a better solution would be: diff --git a/backends/rng-random.c b/backends/rng-random.c index acd20af..9fde566 100644 --- a/backends/rng-random.c +++ b/backends/rng-random.c @@ -41,6 +41,9 @@ static void entropy_available(void *opaque) ssize_t len; len = read(s-fd, buffer, s-size); +if (len == -1 errno == EINTR) { +return; +} That's a typo. I meant s/EINTR/EAGAIN/g Regards, Anthony Liguori This has to be an additional fix on top of this one. EAGAIN has to be handled if we want to allow nonblocking reads, and there doesn't seem to be any reason to have these reads be non-blocking. OTOH, I also think we could use the glib functions for file IO, since handling EINTR in each open-coded read call isn't always fun. Amit
Re: [Qemu-devel] [PATCH v4 0/7] virtio-serial refactoring.
fred.kon...@greensocs.com writes: From: KONRAD Frederic fred.kon...@greensocs.com This is the next part of virtio-refactoring. Basically it creates virtio-serial device which extends virtio-device. Then a virtio-serial can be connected on a virtio-bus. virtio-serial-pci, virtio-serial-s390 and virtio-serial-ccw are created too, they extend respectively virtio-pci, virtio-s390-device, virtio-ccw-device and have a virtio-serial. You can checkout my branch here: git://project.greensocs.com/qemu-virtio.git virtio-serial-v4 Note that it is nearly the same series as virtio-blk and virtio-scsi refactoring. I made basic tests (with linux guests) on: * qemu-system-i386 CHanges v3 - v': * Removed serial configuration field ommited in VirtioCCWDevice structure. Changes v2 - v3: * Added CCW device. * Rebased. Would like to at least see an Acked-by from Amit here... Regards, Anthony Liguori Thanks, Fred KONRAD Frederic (7): virtio-serial: add the virtio-serial device. virtio-serial-pci: switch to the new API. virtio-serial-s390: switch to the new API. virtio-serial-ccw: switch to the new API. virtio-serial: cleanup: init and exit functions. virtio-serial: cleanup: use QOM casts. virtio-serial: cleanup: remove qdev field. hw/s390x/s390-virtio-bus.c | 32 + hw/s390x/s390-virtio-bus.h | 12 +++- hw/s390x/virtio-ccw.c | 30 + hw/s390x/virtio-ccw.h | 12 +++- hw/virtio-pci.c| 130 ++--- hw/virtio-pci.h| 14 +++- hw/virtio-serial-bus.c | 157 +++-- hw/virtio-serial.h | 13 +++- 8 files changed, 240 insertions(+), 160 deletions(-) -- 1.7.11.7
Re: [Qemu-devel] KVM call agenda for 2013-04-02
Juan Quintela quint...@redhat.com wrote: Hi Please send in any agenda topics you are interested in. As there are no items, today call is cancelled. Happy hacking.
Re: [Qemu-devel] vNVRAM / blobstore design
You are of course correct. I advised an integrity value just to detect a hardware or software fault. The check value would not protect against an attack. Fair enough, but why protect these bits specifically? E.g. disk corruption seems more likely (since it's bigger). Add integrity at that level? Why even stop at detection, let's do error correction ... Why ... just because it's a security device. Whenever I code for security, I add layers of protection, constantly looking for this should never happen cases. It might be just a small benefit, but hashing a few kbytes is a small part of TPM startup time, and the function is already there. Think of it as part of the larger (and required) TPM self test that a TPM must do.
[Qemu-devel] [PATCH 2/2] xen-mapcache: pass the right size argument to test_bits
From: Hanweidong hanweid...@huawei.com Compute the correct size for test_bits(). qemu_get_ram_ptr() and qemu_safe_ram_ptr() will call xen_map_cache() with size is 0 if the requested address is in the RAM. Then xen_map_cache() will pass the size 0 to test_bits() for checking if the corresponding pfn was mapped in cache. But test_bits() will always return 1 when size is 0 without any bit testing. Actually, for this case, test_bits should check one bit. So this patch introduced a __test_bit_size which is greater than 0 and a multiple of XC_PAGE_SIZE, then test_bits can work correctly with __test_bit_size XC_PAGE_SHIFT as its size. Signed-off-by: Zhenguo Wang wangzhen...@huawei.com Signed-off-by: Weidong Han hanweid...@huawei.com Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com --- xen-mapcache.c | 26 ++ 1 files changed, 22 insertions(+), 4 deletions(-) diff --git a/xen-mapcache.c b/xen-mapcache.c index a80cbdb..5a626cd 100644 --- a/xen-mapcache.c +++ b/xen-mapcache.c @@ -200,6 +200,7 @@ uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size, hwaddr address_index; hwaddr address_offset; hwaddr __size = size; +hwaddr __test_bit_size = size; bool translated = false; tryagain: @@ -208,9 +209,23 @@ tryagain: trace_xen_map_cache(phys_addr); +/* __test_bit_size is always a multiple of XC_PAGE_SIZE */ +if (size) { +__test_bit_size = size + (phys_addr (XC_PAGE_SIZE - 1)); + +if (__test_bit_size % XC_PAGE_SIZE) { +__test_bit_size += XC_PAGE_SIZE - (__test_bit_size % XC_PAGE_SIZE); +} +} else { +__test_bit_size = XC_PAGE_SIZE; +} + if (mapcache-last_entry != NULL mapcache-last_entry-paddr_index == address_index -!lock !__size) { +!lock !__size +test_bits(address_offset XC_PAGE_SHIFT, + __test_bit_size XC_PAGE_SHIFT, + mapcache-last_entry-valid_mapping)) { trace_xen_map_cache_return(mapcache-last_entry-vaddr_base + address_offset); return mapcache-last_entry-vaddr_base + address_offset; } @@ -229,7 +244,8 @@ tryagain: while (entry entry-lock entry-vaddr_base (entry-paddr_index != address_index || entry-size != __size || - !test_bits(address_offset XC_PAGE_SHIFT, size XC_PAGE_SHIFT, + !test_bits(address_offset XC_PAGE_SHIFT, + __test_bit_size XC_PAGE_SHIFT, entry-valid_mapping))) { pentry = entry; entry = entry-next; @@ -241,13 +257,15 @@ tryagain: } else if (!entry-lock) { if (!entry-vaddr_base || entry-paddr_index != address_index || entry-size != __size || -!test_bits(address_offset XC_PAGE_SHIFT, size XC_PAGE_SHIFT, +!test_bits(address_offset XC_PAGE_SHIFT, +__test_bit_size XC_PAGE_SHIFT, entry-valid_mapping)) { xen_remap_bucket(entry, __size, address_index); } } -if(!test_bits(address_offset XC_PAGE_SHIFT, size XC_PAGE_SHIFT, +if(!test_bits(address_offset XC_PAGE_SHIFT, +__test_bit_size XC_PAGE_SHIFT, entry-valid_mapping)) { mapcache-last_entry = NULL; if (!translated mapcache-phys_offset_to_gaddr) { -- 1.7.2.5
[Qemu-devel] [PATCH 1/2] xen-mapcache: replace last_address_index with a last_entry pointer
Replace last_address_index and last_address_vaddr with a single pointer to the last MapCacheEntry used. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com --- xen-mapcache.c | 34 -- 1 files changed, 16 insertions(+), 18 deletions(-) diff --git a/xen-mapcache.c b/xen-mapcache.c index dc6d1fa..a80cbdb 100644 --- a/xen-mapcache.c +++ b/xen-mapcache.c @@ -74,8 +74,7 @@ typedef struct MapCache { QTAILQ_HEAD(map_cache_head, MapCacheRev) locked_entries; /* For most cases (99.9%), the page address is the same. */ -hwaddr last_address_index; -uint8_t *last_address_vaddr; +MapCacheEntry *last_entry; unsigned long max_mcache_size; unsigned int mcache_bucket_shift; @@ -105,7 +104,6 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void *opaque) mapcache-opaque = opaque; QTAILQ_INIT(mapcache-locked_entries); -mapcache-last_address_index = -1; if (geteuid() == 0) { rlimit_as.rlim_cur = RLIM_INFINITY; @@ -210,9 +208,11 @@ tryagain: trace_xen_map_cache(phys_addr); -if (address_index == mapcache-last_address_index !lock !__size) { -trace_xen_map_cache_return(mapcache-last_address_vaddr + address_offset); -return mapcache-last_address_vaddr + address_offset; +if (mapcache-last_entry != NULL +mapcache-last_entry-paddr_index == address_index +!lock !__size) { +trace_xen_map_cache_return(mapcache-last_entry-vaddr_base + address_offset); +return mapcache-last_entry-vaddr_base + address_offset; } /* size is always a multiple of MCACHE_BUCKET_SIZE */ @@ -249,7 +249,7 @@ tryagain: if(!test_bits(address_offset XC_PAGE_SHIFT, size XC_PAGE_SHIFT, entry-valid_mapping)) { -mapcache-last_address_index = -1; +mapcache-last_entry = NULL; if (!translated mapcache-phys_offset_to_gaddr) { phys_addr = mapcache-phys_offset_to_gaddr(phys_addr, size, mapcache-opaque); translated = true; @@ -259,19 +259,18 @@ tryagain: return NULL; } -mapcache-last_address_index = address_index; -mapcache-last_address_vaddr = entry-vaddr_base; +mapcache-last_entry = entry; if (lock) { MapCacheRev *reventry = g_malloc0(sizeof(MapCacheRev)); entry-lock++; -reventry-vaddr_req = mapcache-last_address_vaddr + address_offset; -reventry-paddr_index = mapcache-last_address_index; +reventry-vaddr_req = mapcache-last_entry-vaddr_base + address_offset; +reventry-paddr_index = mapcache-last_entry-paddr_index; reventry-size = entry-size; QTAILQ_INSERT_HEAD(mapcache-locked_entries, reventry, next); } -trace_xen_map_cache_return(mapcache-last_address_vaddr + address_offset); -return mapcache-last_address_vaddr + address_offset; +trace_xen_map_cache_return(mapcache-last_entry-vaddr_base + address_offset); +return mapcache-last_entry-vaddr_base + address_offset; } ram_addr_t xen_ram_addr_from_mapcache(void *ptr) @@ -338,9 +337,9 @@ void xen_invalidate_map_cache_entry(uint8_t *buffer) QTAILQ_REMOVE(mapcache-locked_entries, reventry, next); g_free(reventry); -if (mapcache-last_address_index == paddr_index) { -mapcache-last_address_index = -1; -mapcache-last_address_vaddr = NULL; +if (mapcache-last_entry != NULL +mapcache-last_entry-paddr_index == paddr_index) { +mapcache-last_entry = NULL; } entry = mapcache-entry[paddr_index % mapcache-nr_buckets]; @@ -404,8 +403,7 @@ void xen_invalidate_map_cache(void) entry-valid_mapping = NULL; } -mapcache-last_address_index = -1; -mapcache-last_address_vaddr = NULL; +mapcache-last_entry = NULL; mapcache_unlock(); } -- 1.7.2.5
Re: [Qemu-devel] [PATCH V3 WIP 3/3] disable vhost_verify_ring_mappings check
On Mon, Apr 01, 2013 at 06:05:47PM -0700, Nicholas A. Bellinger wrote: On Fri, 2013-03-29 at 09:14 +0100, Paolo Bonzini wrote: Il 29/03/2013 03:53, Nicholas A. Bellinger ha scritto: On Thu, 2013-03-28 at 06:13 -0400, Paolo Bonzini wrote: I think it's the right thing to do, but maybe not the right place to do this, need to reset after all IO is done, before ring memory is write protected. Our emails are crossing each other unfortunately, but I want to reinforce this: ring memory is not write protected. Understood. However, AFAICT the act of write protecting these ranges for ROM generates the offending callbacks to vhost_set_memory(). The part that I'm missing is if ring memory is not being write protected by make_bios_readonly_intel(), why are the vhost_set_memory() calls being invoked..? Because mappings change for the region that contains the ring. vhost doesn't know yet that the changes do not affect ring memory, vhost_set_memory() is called exactly to ascertain that. Hi Paolo Co, Here's a bit more information on what is going on with the same cpu_physical_memory_map() failure in vhost_verify_ring_mappings().. So as before, at the point that seabios is marking memory as readonly for ROM in src/shadow.c:make_bios_readonly_intel() with the following call: Calling pci_config_writeb(0x31): bdf: 0x pam: 0x005b the memory API update hook triggers back into vhost_region_del() code, and following occurs: Entering vhost_region_del section: 0x7fd30a213b60 offset_within_region: 0xc size: 2146697216 readonly: 0 vhost_region_del: is_rom: 0, rom_device: 0 vhost_region_del: readable: 1 vhost_region_del: ram_addr 0x0, addr: 0x0 size: 2147483648 vhost_region_del: name: pc.ram Entering vhost_set_memory, section: 0x7fd30a213b60 add: 0, dev-started: 1 Entering verify_ring_mappings: start_addr 0x000c size: 2146697216 verify_ring_mappings: ring_phys 0x0 ring_size: 0 verify_ring_mappings: ring_phys 0x0 ring_size: 0 verify_ring_mappings: ring_phys 0xed000 ring_size: 5124 verify_ring_mappings: calling cpu_physical_memory_map ring_phys: 0xed000 l: 5124 address_space_map: addr: 0xed000, plen: 5124 address_space_map: l: 4096, len: 5124 phys_page_find got PHYS_MAP_NODE_NIL .. address_space_map: section: 0x7fd30fabaed0 memory_region_is_ram: 0 readonly: 0 address_space_map: section: 0x7fd30fabaed0 offset_within_region: 0x0 section size: 18446744073709551615 Unable to map ring buffer for ring 2, l: 4096 So the interesting part is that phys_page_find() is not able to locate the corresponding page for vq-ring_phys: 0xed000 from the vhost_region_del() callback with section-offset_within_region: 0xc.. Is there any case where this would not be considered a bug..? register_multipage : d: 0x7fd30f7d0ed0 section: 0x7fd30a2139b0 register_multipage : d: 0x7fd30f7d0ed0 section: 0x7fd30a2139b0 register_multipage : d: 0x7fd30f7d0ed0 section: 0x7fd30a2139b0 Entering vhost_region_add section: 0x7fd30a213aa0 offset_within_region: 0xc size: 32768 readonly: 1 vhost_region_add: is_rom: 0, rom_device: 0 vhost_region_add: readable: 1 vhost_region_add: ram_addr 0x, addr: 0x 0 size: 2147483648 vhost_region_add: name: pc.ram Entering vhost_set_memory, section: 0x7fd30a213aa0 add: 1, dev-started: 1 Entering verify_ring_mappings: start_addr 0x000c size: 32768 verify_ring_mappings: ring_phys 0x0 ring_size: 0 verify_ring_mappings: ring_phys 0x0 ring_size: 0 verify_ring_mappings: ring_phys 0xed000 ring_size: 5124 verify_ring_mappings: Got !ranges_overlap, skipping register_multipage : d: 0x7fd30f7d0ed0 section: 0x7fd30a2139b0 Entering vhost_region_add section: 0x7fd30a213aa0 offset_within_region: 0xc8000 size: 2146664448 readonly: 0 vhost_region_add: is_rom: 0, rom_device: 0 vhost_region_add: readable: 1 vhost_region_add: ram_addr 0x, addr: 0x 0 size: 2147483648 vhost_region_add: name: pc.ram Entering vhost_set_memory, section: 0x7fd30a213aa0 add: 1, dev-started: 1 Entering verify_ring_mappings: start_addr 0x000c8000 size: 2146664448 verify_ring_mappings: ring_phys 0x0 ring_size: 0 verify_ring_mappings: ring_phys 0x0 ring_size: 0 verify_ring_mappings: ring_phys 0xed000 ring_size: 5124 verify_ring_mappings: calling cpu_physical_memory_map ring_phys: 0xed000 l: 5124 address_space_map: addr: 0xed000, plen: 5124 address_space_map: l: 4096, len: 5124 address_space_map: section: 0x7fd30fabb020 memory_region_is_ram: 1 readonly: 0 address_space_map: section: 0x7fd30fabb020 offset_within_region: 0xc8000 section size: 2146664448 address_space_map: l: 4096, len: 1028 address_space_map: section: 0x7fd30fabb020 memory_region_is_ram: 1 readonly: 0 address_space_map: section: 0x7fd30fabb020 offset_within_region: 0xc8000 section size: 2146664448 address_space_map: Calling qemu_ram_ptr_length: raddr: 0x
Re: [Qemu-devel] [Xen-devel] frequently ballooning results in qemu exit
On Tue, 2 Apr 2013, Hanweidong wrote: -Original Message- From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com] Sent: 2013年4月1日 22:39 To: Hanweidong Cc: Stefano Stabellini; Tim (Xen.org); George Dunlap; Andrew Cooper; Yanqiangjun; qemu-devel@nongnu.org; xen-de...@lists.xen.org; Gonglei (Arei); Anthony Perard; Wangzhenguo Subject: RE: [Qemu-devel] [Xen-devel] frequently ballooning results in qemu exit On Sat, 30 Mar 2013, Hanweidong wrote: -Original Message- From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com] Sent: 2013年3月29日 20:37 To: Hanweidong Cc: Tim (Xen.org); George Dunlap; Andrew Cooper; Yanqiangjun; qemu- de...@nongnu.org; xen-de...@lists.xen.org; Gonglei (Arei); Anthony Perard; Wangzhenguo Subject: Re: [Qemu-devel] [Xen-devel] frequently ballooning results in qemu exit On Mon, 25 Mar 2013, Hanweidong wrote: We fixed this issue by below patch which computed the correct size for test_bits(). qemu_get_ram_ptr() and qemu_safe_ram_ptr() will call xen_map_cache() with size is 0 if the requested address is in the RAM. Then xen_map_cache() will pass the size 0 to test_bits() for checking if the corresponding pfn was mapped in cache. But test_bits() will always return 1 when size is 0 without any bit testing. Actually, for this case, test_bits should check one bit. So this patch introduced a __test_bit_size which is greater than 0 and a multiple of XC_PAGE_SIZE, then test_bits can work correctly with __test_bit_size XC_PAGE_SHIFT as its size. Signed-off-by: Zhenguo Wang wangzhen...@huawei.com Signed-off-by: Weidong Han hanweid...@huawei.com Thanks for the patch and for debugging this difficult problem. The reality is that size is never actually 0: when qemu_get_ram_ptr calls xen_map_cache with size 0, it actually means map until the end of the page. As a consequence test_bits should always test at least 1 bit, like you wrote. Yes, for the case of size is 0, we can just simply set __test_bit_size 1. But for size 0, I think set __test_bit_size to size XC_PAGE_SHIFT is incorrect. If size is not multiple of XC_PAGE_SIZE, then the part of (size % XC_PAGE_SIZE) also needs to test 1 bit. For example size is XC_PAGE_SIZE + 1, then it needs to test 2 bits, but size XC_PAGE_SHIFT is only 1. I was assuming that the size is always page aligned. Looking through the code actually I think that it's better not to rely on this assumption. Looking back at your original patch: We fixed this issue by below patch which computed the correct size for test_bits(). qemu_get_ram_ptr() and qemu_safe_ram_ptr() will call xen_map_cache() with size is 0 if the requested address is in the RAM. Then xen_map_cache() will pass the size 0 to test_bits() for checking if the corresponding pfn was mapped in cache. But test_bits() will always return 1 when size is 0 without any bit testing. Actually, for this case, test_bits should check one bit. So this patch introduced a __test_bit_size which is greater than 0 and a multiple of XC_PAGE_SIZE, then test_bits can work correctly with __test_bit_size XC_PAGE_SHIFT as its size. Signed-off-by: Zhenguo Wang wangzhen...@huawei.com Signed-off-by: Weidong Han hanweid...@huawei.com diff --git a/xen-mapcache.c b/xen-mapcache.c index 31c06dc..bd4a97f 100644 --- a/xen-mapcache.c +++ b/xen-mapcache.c @@ -202,6 +202,7 @@ uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size, hwaddr address_index; hwaddr address_offset; hwaddr __size = size; +hwaddr __test_bit_size = size; bool translated = false; tryagain: @@ -210,7 +211,23 @@ tryagain: trace_xen_map_cache(phys_addr); -if (address_index == mapcache-last_address_index !lock !__size) { +entry = mapcache-entry[address_index % mapcache-nr_buckets]; there is no need to move this line up here, see below +/* __test_bit_size is always a multiple of XC_PAGE_SIZE */ +if (size) { +__test_bit_size = size + (phys_addr (XC_PAGE_SIZE - 1)); + +if (__test_bit_size % XC_PAGE_SIZE) { +__test_bit_size += XC_PAGE_SIZE - (__test_bit_size % XC_PAGE_SIZE); +} +} else { +__test_bit_size = XC_PAGE_SIZE; +} this is OK +if (address_index == mapcache-last_address_index !lock !__size +test_bits(address_offset XC_PAGE_SHIFT, + __test_bit_size XC_PAGE_SHIFT, + entry-valid_mapping)) { trace_xen_map_cache_return(mapcache-last_address_vaddr + address_offset); return mapcache-last_address_vaddr + address_offset; } Unless I am missing something this change is unnecessary: if the mapping is