From d4aa431f945bbbff75129af0cd0b155f988150ba Mon Sep 17 00:00:00 2001 From: Zhang Chen Date: Thu, 1 Dec 2016 14:55:57 +0800 Subject: [PATCH 1/6] docs: Fix description of the sentence Say it in another way to make it easier to understand. Signed-off-by: Zhang Chen Signed-off-by: Eric Blake Signed-off-by: Stefan Weil Signed-off-by: Jason Wang --- docs/colo-proxy.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/colo-proxy.txt b/docs/colo-proxy.txt index 76767cb34f..c4941de198 100644 --- a/docs/colo-proxy.txt +++ b/docs/colo-proxy.txt @@ -158,7 +158,9 @@ secondary. == Usage == -Here, we use demo ip and port discribe more clearly. +Here is an example using demonstration IP and port addresses to more +clearly describe the usage. + Primary(ip:3.3.3.3): -netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown -device e1000,id=e0,netdev=hn0,mac=52:a4:00:12:78:66 From d5aa3e6e0cd6259003790769c448d4fbb5b5c810 Mon Sep 17 00:00:00 2001 From: Ladi Prosek Date: Fri, 6 Jan 2017 09:08:53 +0100 Subject: [PATCH 2/6] net: optimize checksum computation Very simple loop optimization with a significant performance impact. Microbenchmark results, modern x86-64: buffer size | speed up ------------+--------- 1500 | 1.7x 64 | 1.5x 8 | 1.15x Microbenchmark results, POWER7: buffer size | speed up ------------+--------- 1500 | 5x 64 | 3.3x 8 | 1.13x There is a lot of room for further improvement at the expense of code complexity - aligned multibyte reads, LE/BE considerations, architecture-specific optimizations, etc. This patch still keeps things simple and readable. Signed-off-by: Ladi Prosek Reviewed-by: Dmitry Fleytman Signed-off-by: Jason Wang --- net/checksum.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/net/checksum.c b/net/checksum.c index 23323b0760..4da72a6a6c 100644 --- a/net/checksum.c +++ b/net/checksum.c @@ -22,17 +22,22 @@ uint32_t net_checksum_add_cont(int len, uint8_t *buf, int seq) { - uint32_t sum = 0; + uint32_t sum1 = 0, sum2 = 0; int i; - for (i = seq; i < seq + len; i++) { - if (i & 1) { - sum += (uint32_t)buf[i - seq]; - } else { - sum += (uint32_t)buf[i - seq] << 8; - } + for (i = 0; i < len - 1; i += 2) { + sum1 += (uint32_t)buf[i]; + sum2 += (uint32_t)buf[i + 1]; + } + if (i < len) { + sum1 += (uint32_t)buf[i]; + } + + if (seq & 1) { + return sum1 + (sum2 << 8); + } else { + return sum2 + (sum1 << 8); } - return sum; } uint16_t net_checksum_finish(uint32_t sum) From 6ac38ed42b3468e37f9ce681b65c796a3be4e387 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Sun, 8 Jan 2017 12:51:55 +0100 Subject: [PATCH 3/6] m68k: QOMify the MCF Fast Ethernet Controller device When running qemu-system-m68k with the "-net" parameter (for example simply "-net nic -net user"), there is currently a confusing warning message saying: Warning: requested NIC (anonymous, model mcf_fec) was not created (not supported by this machine?) This seems to happen because the MCF NIC has never been adapted to the currently expected QEMU device behavior. Thus let's QOMify the NIC now to get rid of the warning message. Signed-off-by: Thomas Huth Signed-off-by: Jason Wang --- hw/m68k/mcf5208.c | 25 ++++++++++++- hw/net/mcf_fec.c | 79 ++++++++++++++++++++++++++++----------- include/hw/m68k/mcf.h | 4 -- include/hw/m68k/mcf_fec.h | 13 +++++++ 4 files changed, 94 insertions(+), 27 deletions(-) create mode 100644 include/hw/m68k/mcf_fec.h diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c index 3438314c35..bad1d332ed 100644 --- a/hw/m68k/mcf5208.c +++ b/hw/m68k/mcf5208.c @@ -11,6 +11,7 @@ #include "cpu.h" #include "hw/hw.h" #include "hw/m68k/mcf.h" +#include "hw/m68k/mcf_fec.h" #include "qemu/timer.h" #include "hw/ptimer.h" #include "sysemu/sysemu.h" @@ -18,6 +19,7 @@ #include "net/net.h" #include "hw/boards.h" #include "hw/loader.h" +#include "hw/sysbus.h" #include "elf.h" #include "exec/address-spaces.h" @@ -192,6 +194,26 @@ static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic) } } +static void mcf_fec_init(MemoryRegion *sysmem, NICInfo *nd, hwaddr base, + qemu_irq *irqs) +{ + DeviceState *dev; + SysBusDevice *s; + int i; + + qemu_check_nic_model(nd, TYPE_MCF_FEC_NET); + dev = qdev_create(NULL, TYPE_MCF_FEC_NET); + qdev_set_nic_properties(dev, nd); + qdev_init_nofail(dev); + + s = SYS_BUS_DEVICE(dev); + for (i = 0; i < FEC_NUM_IRQ; i++) { + sysbus_connect_irq(s, i, irqs[i]); + } + + memory_region_add_subregion(sysmem, base, sysbus_mmio_get_region(s, 0)); +} + static void mcf5208evb_init(MachineState *machine) { ram_addr_t ram_size = machine->ram_size; @@ -243,9 +265,10 @@ static void mcf5208evb_init(MachineState *machine) fprintf(stderr, "Too many NICs\n"); exit(1); } - if (nd_table[0].used) + if (nd_table[0].used) { mcf_fec_init(address_space_mem, &nd_table[0], 0xfc030000, pic + 36); + } /* 0xfc000000 SCM. */ /* 0xfc004000 XBS. */ diff --git a/hw/net/mcf_fec.c b/hw/net/mcf_fec.c index 4025eb3b33..a3eca7e0f5 100644 --- a/hw/net/mcf_fec.c +++ b/hw/net/mcf_fec.c @@ -9,7 +9,9 @@ #include "hw/hw.h" #include "net/net.h" #include "hw/m68k/mcf.h" +#include "hw/m68k/mcf_fec.h" #include "hw/net/mii.h" +#include "hw/sysbus.h" /* For crc32 */ #include #include "exec/address-spaces.h" @@ -27,9 +29,10 @@ do { printf("mcf_fec: " fmt , ## __VA_ARGS__); } while (0) #define FEC_MAX_FRAME_SIZE 2032 typedef struct { - MemoryRegion *sysmem; + SysBusDevice parent_obj; + MemoryRegion iomem; - qemu_irq *irq; + qemu_irq irq[FEC_NUM_IRQ]; NICState *nic; NICConf conf; uint32_t irq_state; @@ -68,7 +71,6 @@ typedef struct { #define FEC_RESET 1 /* Map interrupt flags onto IRQ lines. */ -#define FEC_NUM_IRQ 13 static const uint32_t mcf_fec_irq_map[FEC_NUM_IRQ] = { FEC_INT_TXF, FEC_INT_TXB, @@ -208,8 +210,10 @@ static void mcf_fec_enable_rx(mcf_fec_state *s) } } -static void mcf_fec_reset(mcf_fec_state *s) +static void mcf_fec_reset(DeviceState *dev) { + mcf_fec_state *s = MCF_FEC_NET(dev); + s->eir = 0; s->eimr = 0; s->rx_enabled = 0; @@ -330,7 +334,7 @@ static void mcf_fec_write(void *opaque, hwaddr addr, s->ecr = value; if (value & FEC_RESET) { DPRINTF("Reset\n"); - mcf_fec_reset(s); + mcf_fec_reset(opaque); } if ((s->ecr & FEC_EN) == 0) { s->rx_enabled = 0; @@ -513,24 +517,55 @@ static NetClientInfo net_mcf_fec_info = { .receive = mcf_fec_receive, }; -void mcf_fec_init(MemoryRegion *sysmem, NICInfo *nd, - hwaddr base, qemu_irq *irq) +static void mcf_fec_realize(DeviceState *dev, Error **errp) { - mcf_fec_state *s; - - qemu_check_nic_model(nd, "mcf_fec"); - - s = (mcf_fec_state *)g_malloc0(sizeof(mcf_fec_state)); - s->sysmem = sysmem; - s->irq = irq; - - memory_region_init_io(&s->iomem, NULL, &mcf_fec_ops, s, "fec", 0x400); - memory_region_add_subregion(sysmem, base, &s->iomem); - - s->conf.macaddr = nd->macaddr; - s->conf.peers.ncs[0] = nd->netdev; - - s->nic = qemu_new_nic(&net_mcf_fec_info, &s->conf, nd->model, nd->name, s); + mcf_fec_state *s = MCF_FEC_NET(dev); + s->nic = qemu_new_nic(&net_mcf_fec_info, &s->conf, + object_get_typename(OBJECT(dev)), dev->id, s); qemu_format_nic_info_str(qemu_get_queue(s->nic), s->conf.macaddr.a); } + +static void mcf_fec_instance_init(Object *obj) +{ + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); + mcf_fec_state *s = MCF_FEC_NET(obj); + int i; + + memory_region_init_io(&s->iomem, obj, &mcf_fec_ops, s, "fec", 0x400); + sysbus_init_mmio(sbd, &s->iomem); + for (i = 0; i < FEC_NUM_IRQ; i++) { + sysbus_init_irq(sbd, &s->irq[i]); + } +} + +static Property mcf_fec_properties[] = { + DEFINE_NIC_PROPERTIES(mcf_fec_state, conf), + DEFINE_PROP_END_OF_LIST(), +}; + +static void mcf_fec_class_init(ObjectClass *oc, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(oc); + + set_bit(DEVICE_CATEGORY_NETWORK, dc->categories); + dc->realize = mcf_fec_realize; + dc->desc = "MCF Fast Ethernet Controller network device"; + dc->reset = mcf_fec_reset; + dc->props = mcf_fec_properties; +} + +static const TypeInfo mcf_fec_info = { + .name = TYPE_MCF_FEC_NET, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(mcf_fec_state), + .instance_init = mcf_fec_instance_init, + .class_init = mcf_fec_class_init, +}; + +static void mcf_fec_register_types(void) +{ + type_register_static(&mcf_fec_info); +} + +type_init(mcf_fec_register_types) diff --git a/include/hw/m68k/mcf.h b/include/hw/m68k/mcf.h index fdae229502..bf43998d9b 100644 --- a/include/hw/m68k/mcf.h +++ b/include/hw/m68k/mcf.h @@ -21,10 +21,6 @@ qemu_irq *mcf_intc_init(struct MemoryRegion *sysmem, hwaddr base, M68kCPU *cpu); -/* mcf_fec.c */ -void mcf_fec_init(struct MemoryRegion *sysmem, NICInfo *nd, - hwaddr base, qemu_irq *irq); - /* mcf5206.c */ qemu_irq *mcf5206_init(struct MemoryRegion *sysmem, uint32_t base, M68kCPU *cpu); diff --git a/include/hw/m68k/mcf_fec.h b/include/hw/m68k/mcf_fec.h new file mode 100644 index 0000000000..7f029f7b59 --- /dev/null +++ b/include/hw/m68k/mcf_fec.h @@ -0,0 +1,13 @@ +/* + * Definitions for the ColdFire Fast Ethernet Controller emulation. + * + * This code is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#define TYPE_MCF_FEC_NET "mcf-fec" +#define MCF_FEC_NET(obj) OBJECT_CHECK(mcf_fec_state, (obj), TYPE_MCF_FEC_NET) + +#define FEC_NUM_IRQ 13 From 581f7b127dfe9ed10ce3216e440cc5be1582911b Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 9 Jan 2017 18:43:57 +0000 Subject: [PATCH 4/6] hw/net/dp8393x: Avoid unintentional sign extensions on addresses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dp8393x has several 32-bit values which are formed by concatenating two 16 bit device register values. Attempting to do these inline with ((s->reg[HI] << 16) | s->reg[LO]) can result in an unintended sign extension because "x << 16" is of type 'int' even though s->reg is unsigned, and so if the expression is used in a context where it is cast to uint64_t the value is incorrectly sign-extended. Fix this by using accessor functions with a uint32_t return type; this also makes the code a bit easier to read. This should fix Coverity issues 1307765, 1307766, 1307767, 1307768. (To avoid having a ctda read function only used in a DPRINTF, we move the DPRINTF down slightly so it can use the ttda function.) Reviewed-by: Laurent Vivier Tested-by: Laurent Vivier Reviewed-by: Hervé Poussineau Signed-off-by: Peter Maydell Signed-off-by: Jason Wang --- hw/net/dp8393x.c | 95 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 68 insertions(+), 27 deletions(-) diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c index 17f0338d1c..efa33ad40a 100644 --- a/hw/net/dp8393x.c +++ b/hw/net/dp8393x.c @@ -174,6 +174,52 @@ typedef struct dp8393xState { AddressSpace as; } dp8393xState; +/* Accessor functions for values which are formed by + * concatenating two 16 bit device registers. By putting these + * in their own functions with a uint32_t return type we avoid the + * pitfall of implicit sign extension where ((x << 16) | y) is a + * signed 32 bit integer that might get sign-extended to a 64 bit integer. + */ +static uint32_t dp8393x_cdp(dp8393xState *s) +{ + return (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP]; +} + +static uint32_t dp8393x_crba(dp8393xState *s) +{ + return (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0]; +} + +static uint32_t dp8393x_crda(dp8393xState *s) +{ + return (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]; +} + +static uint32_t dp8393x_rbwc(dp8393xState *s) +{ + return (s->regs[SONIC_RBWC1] << 16) | s->regs[SONIC_RBWC0]; +} + +static uint32_t dp8393x_rrp(dp8393xState *s) +{ + return (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_RRP]; +} + +static uint32_t dp8393x_tsa(dp8393xState *s) +{ + return (s->regs[SONIC_TSA1] << 16) | s->regs[SONIC_TSA0]; +} + +static uint32_t dp8393x_ttda(dp8393xState *s) +{ + return (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]; +} + +static uint32_t dp8393x_wt(dp8393xState *s) +{ + return s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; +} + static void dp8393x_update_irq(dp8393xState *s) { int level = (s->regs[SONIC_IMR] & s->regs[SONIC_ISR]) ? 1 : 0; @@ -203,8 +249,7 @@ static void dp8393x_do_load_cam(dp8393xState *s) while (s->regs[SONIC_CDC] & 0x1f) { /* Fill current entry */ - address_space_rw(&s->as, - (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP], + address_space_rw(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); s->cam[index][0] = data[1 * width] & 0xff; s->cam[index][1] = data[1 * width] >> 8; @@ -222,8 +267,7 @@ static void dp8393x_do_load_cam(dp8393xState *s) } /* Read CAM enable */ - address_space_rw(&s->as, - (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_CDP], + address_space_rw(&s->as, dp8393x_cdp(s), MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); s->regs[SONIC_CE] = data[0 * width]; DPRINTF("load cam done. cam enable mask 0x%04x\n", s->regs[SONIC_CE]); @@ -242,8 +286,7 @@ static void dp8393x_do_read_rra(dp8393xState *s) /* Read memory */ width = (s->regs[SONIC_DCR] & SONIC_DCR_DW) ? 2 : 1; size = sizeof(uint16_t) * 4 * width; - address_space_rw(&s->as, - (s->regs[SONIC_URRA] << 16) | s->regs[SONIC_RRP], + address_space_rw(&s->as, dp8393x_rrp(s), MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); /* Update SONIC registers */ @@ -292,7 +335,7 @@ static void dp8393x_set_next_tick(dp8393xState *s) return; } - ticks = s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; + ticks = dp8393x_wt(s); s->wt_last_update = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); delay = NANOSECONDS_PER_SECOND * ticks / 5000000; timer_mod(s->watchdog, s->wt_last_update + delay); @@ -309,7 +352,7 @@ static void dp8393x_update_wt_regs(dp8393xState *s) } elapsed = s->wt_last_update - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); - val = s->regs[SONIC_WT1] << 16 | s->regs[SONIC_WT0]; + val = dp8393x_wt(s); val -= elapsed / 5000000; s->regs[SONIC_WT1] = (val >> 16) & 0xffff; s->regs[SONIC_WT0] = (val >> 0) & 0xffff; @@ -356,12 +399,11 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) while (1) { /* Read memory */ - DPRINTF("Transmit packet at %08x\n", - (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_CTDA]); size = sizeof(uint16_t) * 6 * width; s->regs[SONIC_TTDA] = s->regs[SONIC_CTDA]; + DPRINTF("Transmit packet at %08x\n", dp8393x_ttda(s)); address_space_rw(&s->as, - ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * width, + dp8393x_ttda(s) + sizeof(uint16_t) * width, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); tx_len = 0; @@ -386,8 +428,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) if (tx_len + len > sizeof(s->tx_buffer)) { len = sizeof(s->tx_buffer) - tx_len; } - address_space_rw(&s->as, - (s->regs[SONIC_TSA1] << 16) | s->regs[SONIC_TSA0], + address_space_rw(&s->as, dp8393x_tsa(s), MEMTXATTRS_UNSPECIFIED, &s->tx_buffer[tx_len], len, 0); tx_len += len; @@ -396,7 +437,7 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) /* Read next fragment details */ size = sizeof(uint16_t) * 3 * width; address_space_rw(&s->as, - ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * i) * width, + dp8393x_ttda(s) + sizeof(uint16_t) * (4 + 3 * i) * width, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); s->regs[SONIC_TSA0] = data[0 * width]; s->regs[SONIC_TSA1] = data[1 * width]; @@ -430,14 +471,16 @@ static void dp8393x_do_transmit_packets(dp8393xState *s) data[0 * width] = s->regs[SONIC_TCR] & 0x0fff; /* status */ size = sizeof(uint16_t) * width; address_space_rw(&s->as, - (s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA], + dp8393x_ttda(s), MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1); if (!(s->regs[SONIC_CR] & SONIC_CR_HTX)) { /* Read footer of packet */ size = sizeof(uint16_t) * width; address_space_rw(&s->as, - ((s->regs[SONIC_UTDA] << 16) | s->regs[SONIC_TTDA]) + sizeof(uint16_t) * (4 + 3 * s->regs[SONIC_TFC]) * width, + dp8393x_ttda(s) + + sizeof(uint16_t) * + (4 + 3 * s->regs[SONIC_TFC]) * width, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); s->regs[SONIC_CTDA] = data[0 * width] & ~0x1; if (data[0 * width] & 0x1) { @@ -700,7 +743,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, if (s->regs[SONIC_LLFA] & 0x1) { /* Are we still in resource exhaustion? */ size = sizeof(uint16_t) * 1 * width; - address = ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width; + address = dp8393x_crda(s) + sizeof(uint16_t) * 5 * width; address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); if (data[0 * width] & 0x1) { @@ -719,8 +762,8 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, checksum = cpu_to_le32(crc32(0, buf, rx_len)); /* Put packet into RBA */ - DPRINTF("Receive packet at %08x\n", (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0]); - address = (s->regs[SONIC_CRBA1] << 16) | s->regs[SONIC_CRBA0]; + DPRINTF("Receive packet at %08x\n", dp8393x_crba(s)); + address = dp8393x_crba(s); address_space_rw(&s->as, address, MEMTXATTRS_UNSPECIFIED, (uint8_t *)buf, rx_len, 1); address += rx_len; @@ -729,13 +772,13 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, rx_len += 4; s->regs[SONIC_CRBA1] = address >> 16; s->regs[SONIC_CRBA0] = address & 0xffff; - available = (s->regs[SONIC_RBWC1] << 16) | s->regs[SONIC_RBWC0]; + available = dp8393x_rbwc(s); available -= rx_len / 2; s->regs[SONIC_RBWC1] = available >> 16; s->regs[SONIC_RBWC0] = available & 0xffff; /* Update status */ - if (((s->regs[SONIC_RBWC1] << 16) | s->regs[SONIC_RBWC0]) < s->regs[SONIC_EOBC]) { + if (dp8393x_rbwc(s) < s->regs[SONIC_EOBC]) { s->regs[SONIC_RCR] |= SONIC_RCR_LPKT; } s->regs[SONIC_RCR] |= packet_type; @@ -746,20 +789,19 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, } /* Write status to memory */ - DPRINTF("Write status at %08x\n", (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]); + DPRINTF("Write status at %08x\n", dp8393x_crda(s)); data[0 * width] = s->regs[SONIC_RCR]; /* status */ data[1 * width] = rx_len; /* byte count */ data[2 * width] = s->regs[SONIC_TRBA0]; /* pkt_ptr0 */ data[3 * width] = s->regs[SONIC_TRBA1]; /* pkt_ptr1 */ data[4 * width] = s->regs[SONIC_RSC]; /* seq_no */ size = sizeof(uint16_t) * 5 * width; - address_space_rw(&s->as, (s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA], + address_space_rw(&s->as, dp8393x_crda(s), MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 1); /* Move to next descriptor */ size = sizeof(uint16_t) * width; - address_space_rw(&s->as, - ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 5 * width, + address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 5 * width, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, size, 0); s->regs[SONIC_LLFA] = data[0 * width]; if (s->regs[SONIC_LLFA] & 0x1) { @@ -767,8 +809,7 @@ static ssize_t dp8393x_receive(NetClientState *nc, const uint8_t * buf, s->regs[SONIC_ISR] |= SONIC_ISR_RDE; } else { data[0 * width] = 0; /* in_use */ - address_space_rw(&s->as, - ((s->regs[SONIC_URDA] << 16) | s->regs[SONIC_CRDA]) + sizeof(uint16_t) * 6 * width, + address_space_rw(&s->as, dp8393x_crda(s) + sizeof(uint16_t) * 6 * width, MEMTXATTRS_UNSPECIFIED, (uint8_t *)data, sizeof(uint16_t), 1); s->regs[SONIC_CRDA] = s->regs[SONIC_LLFA]; s->regs[SONIC_ISR] |= SONIC_ISR_PKTRX; From a023b7ac6262106e9a7d248bb61ec928fbf94896 Mon Sep 17 00:00:00 2001 From: Alex Kompel Date: Fri, 6 Jan 2017 15:48:27 -0800 Subject: [PATCH 5/6] hw/pci: use-after-free in pci_nic_init_nofail when nic device fails to initialize object_property_set_bool(OBJECT(dev), true, "realized", &err) in pci_nic_init_nofail may release the object if device fails to initialize which leads to use-after-free in error handling block. qdev_init_nofail does the same thing while holding the reference. (gdb) run -net nic qemu-system-x86_64: failed to find romfile "efi-e1000.rom" Program received signal SIGSEGV, Segmentation fault. object_unparent (obj=0x7fffe96a0010) at qom/object.c:440 440 in qom/object.c (gdb) bt , rootbus=0x5555567ed990, default_model=, default_devaddr=) at hw/pci/pci.c:1812 pci_bus=0x5555567ed990) at hw/i386/pc.c:1634 pci_type=0x555555c1a523 "i440FX", host_type=0x555555ba564e "i440FX-pcihost") at hw/i386/pc_piix.c:241 out>, envp=) at vl.c:4481 Signed-off-by: Alex Kompel Signed-off-by: Jason Wang --- hw/pci/pci.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 637d54549e..fe9acecbbf 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1779,7 +1779,6 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, const char *default_devaddr) { const char *devaddr = nd->devaddr ? nd->devaddr : default_devaddr; - Error *err = NULL; PCIBus *bus; PCIDevice *pci_dev; DeviceState *dev; @@ -1805,13 +1804,7 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, pci_dev = pci_create(bus, devfn, pci_nic_names[i]); dev = &pci_dev->qdev; qdev_set_nic_properties(dev, nd); - - object_property_set_bool(OBJECT(dev), true, "realized", &err); - if (err) { - error_report_err(err); - object_unparent(OBJECT(dev)); - exit(1); - } + qdev_init_nofail(dev); return pci_dev; } From fac7d7b1cdb21f921d7ac396365f5e920ef03096 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 10 Jan 2017 19:21:54 +0000 Subject: [PATCH 6/6] tap: fix memory leak on failure in net_init_tap() Commit 091a6b2ac fixed most of the memory leaks in failure paths in net_init_tap() reported by Coverity (CID 1356216), but missed one. Fix it by deferring the allocation of fds and vhost_fds until after the error check. Signed-off-by: Peter Maydell Signed-off-by: Jason Wang --- net/tap.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/net/tap.c b/net/tap.c index b6896a7b7c..6248e8530e 100644 --- a/net/tap.c +++ b/net/tap.c @@ -788,8 +788,8 @@ int net_init_tap(const Netdev *netdev, const char *name, return -1; } } else if (tap->has_fds) { - char **fds = g_new0(char *, MAX_TAP_QUEUES); - char **vhost_fds = g_new0(char *, MAX_TAP_QUEUES); + char **fds; + char **vhost_fds; int nfds, nvhosts; if (tap->has_ifname || tap->has_script || tap->has_downscript || @@ -801,6 +801,9 @@ int net_init_tap(const Netdev *netdev, const char *name, return -1; } + fds = g_new0(char *, MAX_TAP_QUEUES); + vhost_fds = g_new0(char *, MAX_TAP_QUEUES); + nfds = get_fds(tap->fds, fds, MAX_TAP_QUEUES); if (tap->has_vhostfds) { nvhosts = get_fds(tap->vhostfds, vhost_fds, MAX_TAP_QUEUES);