Re: [Qemu-devel] [PATCH for-1.4] Revert "e1000: no need auto-negotiation if link was down"
On Thu, Jan 31, 2013 at 05:43:51PM -0600, Michael Roth wrote: > This reverts commit 84dd2120247a7d25ff1bb337de21c0e76816ad2d. > > I'm not sure what issue the original commit was meant to fix, or if > the logic is actually wrong, but it causes e1000 to stop working > after a guest issues a reset. > > >From what I can tell a guest with an e1000 nic has no way of changing > the link status, as far as it's NetClient peer is concerned, except > in the auto-negotiation path, so with this patch in place there's no > recovery after a reset, since the link goes down and stays that way. > > Revert this patch now to fix the bigger problem, and handle any > lingering issues with a follow-up. > > Reproduced/tested with qemu-jeos and Ubuntu 12.10. > > Signed-off-by: Michael Roth So from discussion on list, it seems clear the proper fix will involve adding more state, which likely will need to be migrated as well. Seems like revert is the best we can do for 1.4. Acked-by: Michael S. Tsirkin > --- > hw/e1000.c |5 - > 1 file changed, 5 deletions(-) > > diff --git a/hw/e1000.c b/hw/e1000.c > index ef06ca1..563a58f 100644 > --- a/hw/e1000.c > +++ b/hw/e1000.c > @@ -166,11 +166,6 @@ static void > set_phy_ctrl(E1000State *s, int index, uint16_t val) > { > if ((val & MII_CR_AUTO_NEG_EN) && (val & MII_CR_RESTART_AUTO_NEG)) { > -/* no need auto-negotiation if link was down */ > -if (s->nic->nc.link_down) { > -s->phy_reg[PHY_STATUS] |= MII_SR_AUTONEG_COMPLETE; > -return; > -} > s->nic->nc.link_down = true; > e1000_link_down(s); > s->phy_reg[PHY_STATUS] &= ~MII_SR_AUTONEG_COMPLETE; > -- > 1.7.9.5 >
Re: [Qemu-devel] [PATCHv2] virtio-net: remove mq feature flag
On 02/04/2013 03:41 PM, Michael S. Tsirkin wrote: > mq flag is not needed: we can look at the number of queues and set > the flag accordingly. > Removing this feature removes ambiguity (what does it mean to have > queues=2 with mq=off?), and simplifies compatibility hacks. > work-around for buggy windows > guests. > > Signed-off-by: Michael S. Tsirkin Looks a nice fix. Acked-by: Jason Wang Tested with windows guest and Linux guest (both mq and sq). So: Tested-by: Jason Wang > --- > hw/pc_piix.c| 4 > hw/virtio-net.c | 8 +++- > hw/virtio-net.h | 15 --- > 3 files changed, 19 insertions(+), 8 deletions(-) > > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 0af436c..ba09714 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c > @@ -313,10 +313,6 @@ static QEMUMachine pc_i440fx_machine_v1_4 = { > .driver = "virtio-net-pci",\ > .property = "ctrl_mac_addr",\ > .value= "off", \ > -},{ \ > -.driver = "virtio-net-pci", \ > -.property = "mq", \ > -.value= "off", \ > } > > static QEMUMachine pc_machine_v1_3 = { > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > index e37358a..8db1787 100644 > --- a/hw/virtio-net.c > +++ b/hw/virtio-net.c > @@ -346,6 +346,10 @@ static uint32_t virtio_net_get_features(VirtIODevice > *vdev, uint32_t features) > > features |= (1 << VIRTIO_NET_F_MAC); > > +if (n->max_queues > 1) { > +features |= (0x1 << VIRTIO_NET_F_MQ); > +} > + > if (!peer_has_vnet_hdr(n)) { > features &= ~(0x1 << VIRTIO_NET_F_CSUM); > features &= ~(0x1 << VIRTIO_NET_F_HOST_TSO4); > @@ -1285,7 +1289,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf > *conf, > int i; > > n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET, > -sizeof(struct virtio_net_config), > +conf->queues > 1 ? > +sizeof(struct virtio_net_config) > : > +sizeof(struct > virtio_net_config_compat), > sizeof(VirtIONet)); > > n->vdev.get_config = virtio_net_get_config; > diff --git a/hw/virtio-net.h b/hw/virtio-net.h > index f5fea6e..b1f7647 100644 > --- a/hw/virtio-net.h > +++ b/hw/virtio-net.h > @@ -69,6 +69,17 @@ typedef struct virtio_net_conf > /* Maximum packet size we can receive from tap device: header + 64k */ > #define VIRTIO_NET_MAX_BUFSIZE (sizeof(struct virtio_net_hdr) + (64 << 10)) > > +/* > + * Windows drivers from 22 Jan 2013 and older fail when config size != 32. > + */ > +struct virtio_net_config_compat > +{ > +/* The config defining mac address ($ETH_ALEN bytes) */ > +uint8_t mac[ETH_ALEN]; > +/* See VIRTIO_NET_F_STATUS and VIRTIO_NET_S_* above */ > +uint16_t status; > +} QEMU_PACKED; > + > struct virtio_net_config > { > /* The config defining mac address ($ETH_ALEN bytes) */ > @@ -190,7 +201,5 @@ struct virtio_net_ctrl_mq { > DEFINE_PROP_BIT("ctrl_rx", _state, _field, VIRTIO_NET_F_CTRL_RX, > true), \ > DEFINE_PROP_BIT("ctrl_vlan", _state, _field, VIRTIO_NET_F_CTRL_VLAN, > true), \ > DEFINE_PROP_BIT("ctrl_rx_extra", _state, _field, > VIRTIO_NET_F_CTRL_RX_EXTRA, true), \ > -DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, > VIRTIO_NET_F_CTRL_MAC_ADDR, true), \ > -DEFINE_PROP_BIT("mq", _state, _field, VIRTIO_NET_F_MQ, true) > - > +DEFINE_PROP_BIT("ctrl_mac_addr", _state, _field, > VIRTIO_NET_F_CTRL_MAC_ADDR, true) > #endif
Re: [Qemu-devel] [PATCHv2] virtio-net: remove mq feature flag
04.02.2013 11:41, Michael S. Tsirkin wrote: mq flag is not needed: we can look at the number of queues and set the flag accordingly. Removing this feature removes ambiguity (what does it mean to have queues=2 with mq=off?), and simplifies compatibility hacks. work-around for buggy windows guests. Signed-off-by: Michael S. Tsirkin Nice fix. I tested it with a few non-mq guests (winXP and win7 with slightly older drivers and with current (but still buggy) drivers), and linux both mq and non-mq - it appears to work just fine. Tested-By: Michael Tokarev With one comment below: @@ -1285,7 +1289,9 @@ VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf, int i; n = (VirtIONet *)virtio_common_init("virtio-net", VIRTIO_ID_NET, -sizeof(struct virtio_net_config), +conf->queues > 1 ? +sizeof(struct virtio_net_config) : +sizeof(struct virtio_net_config_compat), sizeof(VirtIONet)); Please add a comment in this place describing what we're doing. Later on it will be less easy to remember or to find this thread, and the code does not tell what's going on and why this is needed ;) You added a tiny comment around the definition of virtio_net_config_compat struct, but it does not explain why the criteria to enable it is conf->queues being larger than 1. Thank you! /mjt
Re: [Qemu-devel] [PATCH] get_tmp_filename: add explicit error message
On Fri, Feb 01, 2013 at 06:13:51PM +0100, Fabien Chouteau wrote: > > Signed-off-by: Fabien Chouteau > --- > block.c | 13 ++--- > 1 file changed, 10 insertions(+), 3 deletions(-) Hi Fabien, Please always CC qemu-devel@nongnu.org. All patches must be on qemu-devel so that the community can review them - not everyone subscribes to qemu-trivial. Thanks, Stefan > diff --git a/block.c b/block.c > index ba67c0d..3bf8eda 100644 > --- a/block.c > +++ b/block.c > @@ -428,9 +428,16 @@ int get_tmp_filename(char *filename, int size) > /* GetTempFileName requires that its output buffer (4th param) > have length MAX_PATH or greater. */ > assert(size >= MAX_PATH); > -return (GetTempPath(MAX_PATH, temp_dir) > -&& GetTempFileName(temp_dir, "qem", 0, filename) > -? 0 : -GetLastError()); > +if (GetTempPath(MAX_PATH, temp_dir) == 0) { > +fprintf(stderr, "GetTempPath() error: %d\n", GetLastError()); > +return -GetLastError(); > +} > +if (GetTempFileName(temp_dir, "qem", 0, filename) == 0) { > +fprintf(stderr, "GetTempFileName(%s) error: %d\n", temp_dir, > +GetLastError()); > +return -GetLastError(); > +} > +return 0; > #else > int fd; > const char *tmpdir; > -- > 1.7.9.5 >
Re: [Qemu-devel] [PATCHv2] virtio-net: remove mq feature flag
On Mon, Feb 04, 2013 at 09:41:47AM +0200, Michael S. Tsirkin wrote: > mq flag is not needed: we can look at the number of queues and set > the flag accordingly. > Removing this feature removes ambiguity (what does it mean to have > queues=2 with mq=off?), and simplifies compatibility hacks. > work-around for buggy windows > guests. > > Signed-off-by: Michael S. Tsirkin > --- > hw/pc_piix.c| 4 > hw/virtio-net.c | 8 +++- > hw/virtio-net.h | 15 --- > 3 files changed, 19 insertions(+), 8 deletions(-) Reviewed-by: Stefan Hajnoczi
Re: [Qemu-devel] [PATCH for 1.4] target-s390x: Fix wrong comparison in interrupt handling
On Sun, 3 Feb 2013 21:33:16 +0100 Stefan Weil wrote: > gcc with -Wextra complains about an ordered pointer comparison: > > target-s390x/helper.c:660:27: warning: > ordered comparison of pointer with integer zero [-Wextra] > > Obviously the index was missing in the code. > > Signed-off-by: Stefan Weil > --- > > I hope my analysis and the fix is correct - please review. > > For local builds, I always compile with -Wextra. This bug shows that -Wextra > would be good as a default compiler option for QEMU. Of course some extra > warnings must be explicitly disabled then. I'll send a patch for this after > 1.4. > > Regards, > > Stefan W. > > > target-s390x/helper.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/target-s390x/helper.c b/target-s390x/helper.c > index 3180b90..8bd84ef 100644 > --- a/target-s390x/helper.c > +++ b/target-s390x/helper.c > @@ -657,7 +657,7 @@ static void do_io_interrupt(CPUS390XState *env) > cpu_unmap_lowcore(lowcore); > > env->io_index[isc]--; > -if (env->io_index >= 0) { > +if (env->io_index[isc] >= 0) { > disable = 0; > } > break; Oops, obvious bug when you look at it :) Acked-by: Cornelia Huck
Re: [Qemu-devel] [PATCH v2 4/9] slirp: switch to GPollFD
On Sat, Feb 02, 2013 at 12:46:20PM +, Blue Swirl wrote: > On Fri, Feb 1, 2013 at 1:53 PM, Stefan Hajnoczi wrote: > > Slirp uses rfds/wfds/xfds more extensively than other QEMU components. > > > > The rarely-used out-of-band TCP data feature is used. That means we > > need the full table of select(2) to g_poll(3) events: > > > > rfds -> G_IO_IN | G_IO_HUP | G_IO_ERR > > wfds -> G_IO_OUT | G_IO_ERR > > xfds -> G_IO_PRI > > > > I came up with this table by looking at Linux fs/select.c which maps > > select(2) to poll(2) internally. > > > > Another detail to watch out for are the global variables that reference > > rfds/wfds/xfds during slirp_select_poll(). sofcantrcvmore() and > > sofcantsendmore() use these globals to clear fd_set bits. When > > sofcantrcvmore() is called, the wfds bit is cleared so that the write > > handler will no longer be run for this iteration of the event loop. > > > > This actually seems buggy to me since TCP connections can be half-closed > > and we'd still want to handle data in half-duplex fashion. I think the > > real intention is to avoid running the read/write handler when the > > socket has been fully closed. This is indicated with the SS_NOFDREF > > state bit so we now check for it before invoking the TCP write handler. > > Note that UDP/ICMP code paths don't care because they are > > connectionless. > > > > Note that slirp/ has a lot of tabs and sometimes mixed tabs with spaces. > > I followed the style of the surrounding code. > > Nack for CODING_STYLE. Please either convert the functions affected to > spaces first (as you yourself proposed), or just ignore the > surrounding code and use spaces. Read my lips: no new tabses. Given the project status of upstream slirp, I don't expect us to sync code changes much or at all. Therefore let's convert the file to QEMU coding style. Stefan
Re: [Qemu-devel] [buildbot patch 4/6] use --disable-debug-info
On Fri, Feb 01, 2013 at 03:02:23PM +0100, Gerd Hoffmann wrote: > Signed-off-by: Gerd Hoffmann > --- > qemu-master.cfg |6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) What is the rationale for these changes: --disable-debug-info to save buildslave disk space? Remove CFLAGS=-O2? > diff --git a/qemu-master.cfg b/qemu-master.cfg > index dc5a7a8..2502ced 100644 > --- a/qemu-master.cfg > +++ b/qemu-master.cfg > @@ -696,9 +696,11 @@ def create_build_factory(repourl, branch="HEAD", > env={'LANG': 'C'})) > workdir = os.path.join(workdir, 'outoftree') > configure = "../configure" > -f.addStep(Configure(command=[configure] + list(configure_args), > +f.addStep(Configure(command=[configure, > + "--disable-debug-info"] + > +list(configure_args), > env={'LANG': 'C'}, workdir=workdir)) > -f.addStep(Compile(command=[make, "CFLAGS=-O2"], > +f.addStep(Compile(command=[make], >env={'LANG': 'C'}, >timeout=2400, >workdir=workdir)) > -- > 1.7.9.7 > >
Re: [Qemu-devel] [RFC][PATCH]Add timestamp to error message
On Sat, Feb 02, 2013 at 08:31:45AM +0100, Markus Armbruster wrote: > Seiji Aguchi writes: > > > [Issue] > > When we offer a customer support service and a problem happens > > in a customer's system, we try to understand the problem by > > comparing what the customer reports with message logs of the > > customer's system. > > > > In this case, we often need to know when the problem happens. > > > > But, currently, there is no timestamp in qemu's error messages. > > Therefore, we may not be able to understand the problem based on > > error messages. > > > > [Solution] > > This patch adds a timestamp to qemu's error message logged by > > error_report(). > > > > A logic calculating a time is copied from libvirt, src/util/virtime.c. > > Do we really want to add timestamps to error messages unconditionally? > I don't doubt it's useful in your scenario, but most of the time it's > just annoying clutter. Agreed, I think it should be an option. Also remember that management tools can pass a pipe as stderr when starting QEMU, and then they can add their preferred timestamping/log formatting outside of QEMU. Stefan
Re: [Qemu-devel] [PATCH for-1.4] Revert "e1000: no need auto-negotiation if link was down"
On Thu, Jan 31, 2013 at 05:43:51PM -0600, Michael Roth wrote: > This reverts commit 84dd2120247a7d25ff1bb337de21c0e76816ad2d. > > I'm not sure what issue the original commit was meant to fix, or if > the logic is actually wrong, but it causes e1000 to stop working > after a guest issues a reset. > > From what I can tell a guest with an e1000 nic has no way of changing > the link status, as far as it's NetClient peer is concerned, except > in the auto-negotiation path, so with this patch in place there's no > recovery after a reset, since the link goes down and stays that way. > > Revert this patch now to fix the bigger problem, and handle any > lingering issues with a follow-up. > > Reproduced/tested with qemu-jeos and Ubuntu 12.10. > > Signed-off-by: Michael Roth > --- > hw/e1000.c |5 - > 1 file changed, 5 deletions(-) Acked-by: Stefan Hajnoczi
[Qemu-devel] Question about default floppy and stdvga memory
I tried to disable "default floppy" without use -nodefaults option that disable other things. I didn't found other parameters to do that in docs and code for now. Can someone tell me if there is another way to disable default floppy only? About stdvga, I found that memory is configurable from docs/spec/standard-vga: Framebuffer memory, 16 MB in size (by default). Size is tunable via vga_mem_mb property. vga_mem_mb seems wrong in doc, I didn't found it in the code with grep so I tried to use -global vga.vram_size_mb, that (looking at code) seems the correct one. By default xen reserve 8 mb of videoram but stdvga default is 16 mb. Tried out a domU with Xen, if the memory required from vga is more than memory reserved by xen it should give an error (like qxl and cirrus), but with stdvga I cannot get any error at all. I know that in xen there are changes and/or bugfix (probably in hvmloader) to have the emulated vgas full working with qemu upstream but probably stdvga has something to fix and/or improve also on qemu part. Can someone tell me if memory setting of stdvga is working? Is correct to have no errors from xen even if it reserves only 8 mb by default and this size is not setted on qemu with -global? smime.p7s Description: Firma crittografica S/MIME
Re: [Qemu-devel] How many msi-x vectors should be allocated for the virtio-serial device?
Il 03/02/2013 13:11, Michael S. Tsirkin ha scritto: >> > static Property virtio_serial_properties[] = { >> > DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, >> > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), >> > -DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, >> > DEV_NVECTORS_UNSPECIFIED), >> > +DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), >> > DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), >> > DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), >> > DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, >> > serial.max_virtserial_ports, 31), >> > >> > plus the backwards-compatibility stuff. >> > >> > Paolo > Makes sense, but the logic in virtio_serial_init_pci is > then dead code and should go away. > It won't be dead code for the backwards-compatible machine types (that use DEV_NVECTORS_UNSPECIFIED). Paolo
Re: [Qemu-devel] How many msi-x vectors should be allocated for the virtio-serial device?
On Mon, Feb 04, 2013 at 10:42:32AM +0100, Paolo Bonzini wrote: > Il 03/02/2013 13:11, Michael S. Tsirkin ha scritto: > >> > static Property virtio_serial_properties[] = { > >> > DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags, > >> > VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true), > >> > -DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, > >> > DEV_NVECTORS_UNSPECIFIED), > >> > +DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 2), > >> > DEFINE_PROP_HEX32("class", VirtIOPCIProxy, class_code, 0), > >> > DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features), > >> > DEFINE_PROP_UINT32("max_ports", VirtIOPCIProxy, > >> > serial.max_virtserial_ports, 31), > >> > > >> > plus the backwards-compatibility stuff. > >> > > >> > Paolo > > Makes sense, but the logic in virtio_serial_init_pci is > > then dead code and should go away. > > > > It won't be dead code for the backwards-compatible machine types (that > use DEV_NVECTORS_UNSPECIFIED). > > Paolo Good point. Ack. Want to post this with proper signature etc? -- MST
[Qemu-devel] [PATCH v3] hw: add WM8731 codec support
From: Kuo-Jung Su Wolfson WM8731 is a simple audio codec for embedded systems. It has 2 input and 1 output ports: ** Input ** 1. Linue-In 2. Microphone ** Output ** 1. Headphone out BTW it's based on hw/wm8750.c with 16bit I2S support by default. Signed-off-by: Kuo-Jung Su Cc: Andreas --- Changes for v3: - Update struct name and its typedef to CamelCase. - Add 'const' prior to TypeInfo wm8731_info. - Replace 'WM8731State *s = FROM_I2C_SLAVE(xxx);' with 'WM8731State *s = WM8731(i2c)'. - Remove QOM in critical path (i.e. audio callback). however it turns out that the performance bottleneck is the I2S (ftssp010) and DMA (ftapbbrg020); the DMA hardware handshake is simply too slow in QEMU, it has to perform an DMA HW handshake at each 16-bit word. Changes for v2: - introduce QOM coding style - coding style fixes, howeve there are still some false alarms. for examples: ERROR: need consistent spacing around '*' (ctx:WxV) #9997: FILE: hw/wm8731.c:33: +SWVoiceIn *adc_voice[IN_PORT_N]; default-configs/arm-softmmu.mak |1 + hw/Makefile.objs|1 + hw/i2c.h|6 + hw/wm8731.c | 499 +++ 4 files changed, 507 insertions(+) create mode 100644 hw/wm8731.c diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak index 2f1a5c9..c86d0f2 100644 --- a/default-configs/arm-softmmu.mak +++ b/default-configs/arm-softmmu.mak @@ -10,6 +10,7 @@ CONFIG_SERIAL=y CONFIG_PTIMER=y CONFIG_SD=y CONFIG_MAX7310=y +CONFIG_WM8731=y CONFIG_WM8750=y CONFIG_TWL92230=y CONFIG_TSC2005=y diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 447e32a..3d67cdf 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -165,6 +165,7 @@ common-obj-$(CONFIG_REALLY_VIRTFS) += 9pfs/ common-obj-y += usb/ common-obj-$(CONFIG_PTIMER) += ptimer.o common-obj-$(CONFIG_MAX7310) += max7310.o +common-obj-$(CONFIG_WM8731) += wm8731.o common-obj-$(CONFIG_WM8750) += wm8750.o common-obj-$(CONFIG_TWL92230) += twl92230.o common-obj-$(CONFIG_TSC2005) += tsc2005.o diff --git a/hw/i2c.h b/hw/i2c.h index 883b5c5..89d63cd 100644 --- a/hw/i2c.h +++ b/hw/i2c.h @@ -64,6 +64,12 @@ int i2c_recv(i2c_bus *bus); DeviceState *i2c_create_slave(i2c_bus *bus, const char *name, uint8_t addr); +/* wm8731.c */ +void wm8731_data_req_set(DeviceState *dev, +void (*data_req)(void *, int, int), void *opaque); +void wm8731_dac_dat(void *opaque, uint32_t sample); +uint32_t wm8731_adc_dat(void *opaque); + /* wm8750.c */ void wm8750_data_req_set(DeviceState *dev, void (*data_req)(void *, int, int), void *opaque); diff --git a/hw/wm8731.c b/hw/wm8731.c new file mode 100644 index 000..bd34d33 --- /dev/null +++ b/hw/wm8731.c @@ -0,0 +1,499 @@ +/* + * WM8731 audio codec. + * + * base is wm8750.c + * + * Copyright (c) 2013 Faraday Technology + * Written by Dante Su + * + * This file is licensed under GNU GPL v2+. + */ + +#include "hw.h" +#include "i2c.h" +#include "audio/audio.h" + +#define IN_PORT_N 2 +#define OUT_PORT_N 1 + +#define TYPE_WM8731 "wm8731" + +typedef struct WMRate { +int adc; +int adc_hz; +int dac; +int dac_hz; +} WMRate; + +typedef struct WM8731State { +I2CSlave i2c; +uint8_t i2c_data[2]; +int i2c_len; +QEMUSoundCard card; +SWVoiceIn *adc_voice[IN_PORT_N]; +SWVoiceOut *dac_voice[OUT_PORT_N]; +void (*data_req)(void *, int, int); +void *opaque; +uint8_t data_in[4096]; +uint8_t data_out[4096]; +int idx_in, req_in; +int idx_out, req_out; + +SWVoiceOut **out[2]; +uint8_t outvol[2]; +SWVoiceIn **in[2]; +uint8_t invol[2], inmute[2], mutemic; + +uint8_t mute; +uint8_t power, format, active; +const WMRate *rate; +uint8_t rate_vmstate; +int adc_hz, dac_hz, ext_adc_hz, ext_dac_hz, master; +} WM8731State; + +#define WM8731(obj) \ +OBJECT_CHECK(WM8731State, obj, TYPE_WM8731) + +#define WM8731_OUTVOL_TRANSFORM(x) (x << 1) +#define WM8731_INVOL_TRANSFORM(x) (x << 3) + +static inline void wm8731_in_load(WM8731State *s) +{ +if (s->idx_in + s->req_in <= sizeof(s->data_in)) { +return; +} +s->idx_in = audio_MAX(0, (int) sizeof(s->data_in) - s->req_in); +AUD_read(*s->in[0], s->data_in + s->idx_in, + sizeof(s->data_in) - s->idx_in); +} + +static inline void wm8731_out_flush(WM8731State *s) +{ +int sent = 0; +while (sent < s->idx_out) { +sent += AUD_write(*s->out[0], s->data_out + sent, s->idx_out - sent) +? 0 : s->idx_out; +} +s->idx_out = 0; +} + +static void wm8731_audio_in_cb(void *opaque, int avail_b) +{ +WM8731State *s = opaque; +s->req_in = avail_b; +/* 16 bit samples */ +s->data_req(s->opaque, s->req_out >> 1, avail_b >> 1); +} + +static void wm8731_audio_out_cb(void *opaque, int free_b) +{ +WM8731State *s = opaque; + +if (s-
[Qemu-devel] [PATCH v3] hw/m25p80.c: add WRSR(0x01) support
From: Kuo-Jung Su Atmel, SST and Intel/Numonyx serial flash tend to power up with the software protection bits set. And thus the new m25p80.c in linux kernel would always tries to use WREN(0x06) + WRSR(0x01) to turn-off the protection. The WEL(0x02) of status register is supposed to be cleared after WRSR(0x01). There are also some drivers (i.e mine for RTOSes) would check the WEL(0x02) in status register to make sure the protection is correctly turned off. Signed-off-by: Kuo-Jung Su Cc: Peter Crosthwaite Cc: Peter Maydell Cc: Edgar E. Iglesias --- Changes for v3: - remove s->state = STATE_IDLE in complete_collecting_data() Changes for v2: - coding style fix hw/m25p80.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/hw/m25p80.c b/hw/m25p80.c index 788c196..461b41c 100644 --- a/hw/m25p80.c +++ b/hw/m25p80.c @@ -184,6 +184,7 @@ static const FlashPartInfo known_devices[] = { typedef enum { NOP = 0, +WRSR = 0x1, WRDI = 0x4, RDSR = 0x5, WREN = 0x6, @@ -379,6 +380,11 @@ static void complete_collecting_data(Flash *s) case ERASE_SECTOR: flash_erase(s, s->cur_addr, s->cmd_in_progress); break; +case WRSR: +if (s->write_enable) { +s->write_enable = false; +} +break; default: break; } @@ -443,6 +449,15 @@ static void decode_new_cmd(Flash *s, uint32_t value) s->state = STATE_COLLECTING_DATA; break; +case WRSR: +if (s->write_enable) { +s->needed_bytes = 1; +s->pos = 0; +s->len = 0; +s->state = STATE_COLLECTING_DATA; +} +break; + case WRDI: s->write_enable = false; break; -- 1.7.9.5
[Qemu-devel] [Bug 1042388] Re: qemu: Unsupported syscall: 257 (timer_create)
** Changed in: qemu Status: New => Confirmed -- 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] [buildbot patch 4/6] use --disable-debug-info
On 02/04/13 10:26, Stefan Hajnoczi wrote: > On Fri, Feb 01, 2013 at 03:02:23PM +0100, Gerd Hoffmann wrote: >> Signed-off-by: Gerd Hoffmann >> --- >> qemu-master.cfg |6 -- >> 1 file changed, 4 insertions(+), 2 deletions(-) > > What is the rationale for these changes: > > --disable-debug-info to save buildslave disk space? Yes. > Remove CFLAGS=-O2? That was used to remove '-g' before we had --disable-debug-info, so it isn't needed any more. So there doesn't change much, except that the "make check" tests now are build without debug info too. cheers, Gerd
[Qemu-devel] [PATCH v3] target-arm: add Faraday ARMv5TE processors support
From: Kuo-Jung Su Faraday processors are a series of ARMv4/ARMv5TE clone. * ARMv4 series (FA526, FA626). All of them are now out-of-date, so I have no plan for them. * ARMv5TE series (FA606TE, FA626TE, FA616TE, FA726TE) All the single core RISC listed above are included in this patch. And there are two Faraday CP15 extensions (AUX and I/D-Scratchpad) have been implemented as a read/write value without any extra actions. Signed-off-by: Kuo-Jung Su Cc: Paul Brook Cc: Peter Maydell --- Partial version of Faraday ARM cores are available here: https://docs.google.com/folder/d/0BwfiewvSmUgAalh5TkxyZWtlWEE/edit Changes for v3: - Replace Faraday i/d scratchpad with TCM, since they're compatible. The TCM_STATUS would be redirected to 'TCMTR' in id_cp_reginfo[], and it's always a 'ZERO' which indicates no TCM in QEMU model. - Replace Faraday auxiliary control for FA626TE with ARM AUXCR. - Add Faraday MPU support for FA606TE. - Update CPU_SAVE_VERSION from 9 to 10. - Update machine.c to include c15_tcm_. Changes for v2: - coding style fixes. - create a register cache(field) in cp15 for R/W, rather than NOP. thanks to Paul for the idea. target-arm/cpu.c | 52 +++ target-arm/cpu.h |6 +++- target-arm/helper.c | 84 ++ target-arm/machine.c |4 +++ 4 files changed, 145 insertions(+), 1 deletion(-) diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 1c6a628..d308dbe 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -233,6 +233,54 @@ static void arm926_initfn(Object *obj) cpu->reset_sctlr = 0x00090078; } +static void fa606te_initfn(Object *obj) +{ +ARMCPU *cpu = ARM_CPU(obj); +set_feature(&cpu->env, ARM_FEATURE_V5); +set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS); +set_feature(&cpu->env, ARM_FEATURE_MPU_FARADAY); +cpu->midr = 0x66056061; /* CR0-0 Identification Code Register (ID) */ +cpu->ctr = 0x; /* CR0-1 Cache Type Register (CTR) */ +cpu->reset_sctlr = 0x0078; /* CR1-0 Configuration Register (CFG) */ +} + +static void fa616te_initfn(Object *obj) +{ +ARMCPU *cpu = ARM_CPU(obj); +set_feature(&cpu->env, ARM_FEATURE_V5); +set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS); +set_feature(&cpu->env, ARM_FEATURE_CACHE_TEST_CLEAN); +set_feature(&cpu->env, ARM_FEATURE_TCM_FARADAY); +cpu->midr = 0x66056161; /* CR0-0 Identification Code Register (ID) */ +cpu->ctr = 0x1d152152; /* CR0-1 Cache Type Register (CTR) */ +cpu->reset_sctlr = 0x00050078; /* CR1-0 Configuration Register (CFG) */ +} + +static void fa626te_initfn(Object *obj) +{ +ARMCPU *cpu = ARM_CPU(obj); +set_feature(&cpu->env, ARM_FEATURE_V5); +set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS); +set_feature(&cpu->env, ARM_FEATURE_CACHE_TEST_CLEAN); +set_feature(&cpu->env, ARM_FEATURE_TCM_FARADAY); +set_feature(&cpu->env, ARM_FEATURE_AUXCR); +cpu->midr = 0x66056261; /* CR0-0 Identification Code Register (ID) */ +cpu->ctr = 0x0f192192; /* CR0-1 Cache Type Register (CTR) */ +cpu->reset_sctlr = 0x0078; /* CR1-0 Configuration Register (CFG) */ +} + +static void fa726te_initfn(Object *obj) +{ +ARMCPU *cpu = ARM_CPU(obj); +set_feature(&cpu->env, ARM_FEATURE_V5); +set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS); +set_feature(&cpu->env, ARM_FEATURE_CACHE_TEST_CLEAN); +set_feature(&cpu->env, ARM_FEATURE_TCM_FARADAY); +cpu->midr = 0x66057261; /* CR0-0 Identification Code Register (ID) */ +cpu->ctr = 0x1d192192; /* CR0-1 Cache Type Register (CTR) */ +cpu->reset_sctlr = 0x00050078; /* CR1-0 Configuration Register (CFG) */ +} + static void arm946_initfn(Object *obj) { ARMCPU *cpu = ARM_CPU(obj); @@ -745,6 +793,10 @@ typedef struct ARMCPUInfo { static const ARMCPUInfo arm_cpus[] = { { .name = "arm926", .initfn = arm926_initfn }, +{ .name = "fa606te", .initfn = fa606te_initfn }, +{ .name = "fa616te", .initfn = fa616te_initfn }, +{ .name = "fa626te", .initfn = fa626te_initfn }, +{ .name = "fa726te", .initfn = fa726te_initfn }, { .name = "arm946", .initfn = arm946_initfn }, { .name = "arm1026", .initfn = arm1026_initfn }, /* What QEMU calls "arm1136-r2" is actually the 1136 r0p2, i.e. an diff --git a/target-arm/cpu.h b/target-arm/cpu.h index ffddfcb..e0b0a78 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -152,6 +152,8 @@ typedef struct CPUARMState { uint32_t c15_diagnostic; /* diagnostic register */ uint32_t c15_power_diagnostic; uint32_t c15_power_control; /* power control */ +uint32_t c15_tcm_data; /* Data TCM region register */ +uint32_t c15_tcm_inst; /* Instruction TCM region register */ } cp15; struct { @@ -391,6 +393,8 @@ enum arm_features { ARM_FEATURE_MPIDR, /* has cp15 MPIDR */ AR
Re: [Qemu-devel] [PATCH] vnc: recognize Hungarian doubleacutes
On 02/03/13 21:36, Michael Tokarev wrote: > { "Zabovedot",0x1af}, > { "zacute", 0x1bc}, > { "Zacute", 0x1ac}, > +{ "Odoubleacute", 0x1d5}, Ő > +{ "Udoubleacute", 0x1db}, Ű > { "cacute", 0x1e6}, > { "Cacute", 0x1c6}, > { "nacute", 0x1f1}, > { "Nacute", 0x1d1}, > +{ "odoubleacute", 0x1f5}, ő > +{ "udoubleacute", 0x1fb}, ű The name & low byte of each added code matches the corresponding name & octa value I have in my ~/.XCompose. (I'm Hungarian.) Reviewed-by: Laszlo Ersek
Re: [Qemu-devel] [PATCH] get_tmp_filename: add explicit error message
Stefan Hajnoczi writes: > On Fri, Feb 01, 2013 at 06:13:51PM +0100, Fabien Chouteau wrote: >> >> Signed-off-by: Fabien Chouteau >> --- >> block.c | 13 ++--- >> 1 file changed, 10 insertions(+), 3 deletions(-) > > Hi Fabien, > Please always CC qemu-devel@nongnu.org. All patches must be on > qemu-devel so that the community can review them - not everyone > subscribes to qemu-trivial. > > Thanks, > Stefan > >> diff --git a/block.c b/block.c >> index ba67c0d..3bf8eda 100644 >> --- a/block.c >> +++ b/block.c >> @@ -428,9 +428,16 @@ int get_tmp_filename(char *filename, int size) >> /* GetTempFileName requires that its output buffer (4th param) >> have length MAX_PATH or greater. */ >> assert(size >= MAX_PATH); >> -return (GetTempPath(MAX_PATH, temp_dir) >> -&& GetTempFileName(temp_dir, "qem", 0, filename) >> -? 0 : -GetLastError()); >> +if (GetTempPath(MAX_PATH, temp_dir) == 0) { >> +fprintf(stderr, "GetTempPath() error: %d\n", GetLastError()); >> +return -GetLastError(); >> +} >> +if (GetTempFileName(temp_dir, "qem", 0, filename) == 0) { >> +fprintf(stderr, "GetTempFileName(%s) error: %d\n", temp_dir, >> +GetLastError()); >> +return -GetLastError(); >> +} >> +return 0; >> #else >> int fd; >> const char *tmpdir; get_tmp_filename() is not supposed to print to stderr, that's the caller's job.
Re: [Qemu-devel] Question about default floppy and stdvga memory
Fabio Fantoni writes: > I tried to disable "default floppy" without use -nodefaults option > that disable other things. > I didn't found other parameters to do that in docs and code for now. > Can someone tell me if there is another way to disable default floppy only? As far as I know, there is none. [...]
[Qemu-devel] Patch queue for qemu-1.1.3 stable release
Hello. This is my attempt at releasing a next stable/bugfix release of abandomed 1.1 series of qemu. I tried my best to colledt all fixes, and the last 1.1 has been out for quite a while now, so it think it is time to release 1.1.3 or not do it at all. Several distributions are using 1.1 in their stable branches. I know at least Debian (I maintain qemu package there these days) and Gentoo uses it, maybe also Ubuntu. The current patch queue is available at http://git.corpit.ru/?p=qemu.git;a=shortlog;h=refs/heads/stable-1.1-queue and consists of 60 patches, all will be sent as a reply to this email. If you know any omissions which should be there but aren't, or have other comments of any sort, please tell me. Also, provided there's an agreement, I'd be glad to upload this release (tarball and git branch) to official qemu.org site as well. Thanks, /mjt
[Qemu-devel] [PATCH 03/60] use --libexecdir instead of ignoring it first and reinventing it later
Commit 7b93fadf3a38d1ed65ea5536a52efc2772c6e3b8 "Add basic version of bridge helper" put the bridge helper executable into a fixed ${prefix}/libexec/ location, instead of using ${libexecdir} for this. At the same time, --libexecdir is being happily ignored by ./configure. Even more, the same patch sets unused $libexecdir variable in the generated config-host.mak, and uses fixed string (\${prefix}/libexecdir) for the bridge helper binary. Fix this braindamage by introducing $libexecdir variable, using it for the bridge helper binary, and recognizing --libexecdir. This patch is applicable to stable-1.1. Reviewed-by: Andreas Färber Reviewed-by: Corey Bryant Signed-off-by: Michael Tokarev Cc: Corey Bryant Cc: Richa Marwaha Cc: qemu-sta...@nongnu.org Signed-off-by: Anthony Liguori (cherry picked from commit 8bf188aa18ef7a8355d9edbd43871d590468c4ed) --- configure | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/configure b/configure index df29c2f..1769e5f 100755 --- a/configure +++ b/configure @@ -159,6 +159,7 @@ datadir="\${prefix}/share" qemu_docdir="\${prefix}/share/doc/qemu" bindir="\${prefix}/bin" libdir="\${prefix}/lib" +libexecdir="\${prefix}/libexec" includedir="\${prefix}/include" sysconfdir="\${prefix}/etc" confsuffix="/qemu" @@ -597,6 +598,8 @@ for opt do ;; --libdir=*) libdir="$optarg" ;; + --libexecdir=*) libexecdir="$optarg" + ;; --includedir=*) includedir="$optarg" ;; --datadir=*) datadir="$optarg" @@ -607,7 +610,7 @@ for opt do ;; --sysconfdir=*) sysconfdir="$optarg" ;; - --sbindir=*|--libexecdir=*|--sharedstatedir=*|--localstatedir=*|\ + --sbindir=*|--sharedstatedir=*|--localstatedir=*|\ --oldincludedir=*|--datarootdir=*|--infodir=*|--localedir=*|\ --htmldir=*|--dvidir=*|--pdfdir=*|--psdir=*) # These switches are silently ignored, for compatibility with @@ -2958,6 +2961,7 @@ echo "Install prefix$prefix" echo "BIOS directory`eval echo $qemu_datadir`" echo "binary directory `eval echo $bindir`" echo "library directory `eval echo $libdir`" +echo "libexec directory `eval echo $libexecdir`" echo "include directory `eval echo $includedir`" echo "config directory `eval echo $sysconfdir`" if test "$mingw32" = "no" ; then @@ -3061,14 +3065,14 @@ echo all: >> $config_host_mak echo "prefix=$prefix" >> $config_host_mak echo "bindir=$bindir" >> $config_host_mak echo "libdir=$libdir" >> $config_host_mak +echo "libexecdir=$libexecdir" >> $config_host_mak echo "includedir=$includedir" >> $config_host_mak echo "mandir=$mandir" >> $config_host_mak echo "sysconfdir=$sysconfdir" >> $config_host_mak echo "qemu_confdir=$qemu_confdir" >> $config_host_mak echo "qemu_datadir=$qemu_datadir" >> $config_host_mak echo "qemu_docdir=$qemu_docdir" >> $config_host_mak -echo "libexecdir=\${prefix}/libexec" >> $config_host_mak -echo "CONFIG_QEMU_HELPERDIR=\"$prefix/libexec\"" >> $config_host_mak +echo "CONFIG_QEMU_HELPERDIR=\"$libexecdir\"" >> $config_host_mak echo "ARCH=$ARCH" >> $config_host_mak if test "$debug_tcg" = "yes" ; then -- 1.7.10.4
[Qemu-devel] [PATCH 35/60] s390x: fix -initrd in virtio machine
From: Alexander Graf When using -initrd in the virtio machine, we need to indicate the initrd start and size inside the kernel image. These parameters need to be stored in native endianness. Signed-off-by: Alexander Graf Acked-by: Richard Henderson Acked-by: Christian Borntraeger (cherry picked from commit 235a3f0bed3584fe65079ffa07c7a842971f261e) Signed-off-by: Michael Tokarev --- hw/s390-virtio.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/s390-virtio.c b/hw/s390-virtio.c index c0e19fd..05058c3 100644 --- a/hw/s390-virtio.c +++ b/hw/s390-virtio.c @@ -282,8 +282,8 @@ static void s390_init(ram_addr_t my_ram_size, } /* we have to overwrite values in the kernel image, which are "rom" */ -memcpy(rom_ptr(INITRD_PARM_START), &initrd_offset, 8); -memcpy(rom_ptr(INITRD_PARM_SIZE), &initrd_size, 8); +stq_p(rom_ptr(INITRD_PARM_START), initrd_offset); +stq_p(rom_ptr(INITRD_PARM_SIZE), initrd_size); } if (rom_ptr(KERN_PARM_AREA)) { -- 1.7.10.4
[Qemu-devel] [PATCH 53/60] hw/qxl: qxl_send_events: nop if stopped
From: Alon Levy Added a trace point for easy logging. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=870972 Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann (cherry picked from commit 511aefb0c60e3063ead76d4ba6aabf619eed18ef) Conflicts: hw/qxl.c trace-events Cc: Doug Goldstein Signed-off-by: Michael Tokarev --- hw/qxl.c |8 +++- trace-events |1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/qxl.c b/hw/qxl.c index b971ac7..ed70e6b 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1514,7 +1514,13 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t events) uint32_t old_pending; uint32_t le_events = cpu_to_le32(events); -assert(d->ssd.running); +if (!d->ssd.running) { +/* spice-server tracks guest running state and should not do this */ +fprintf(stderr, "%s: spice-server bug: guest stopped, ignoring\n", +__func__); +trace_qxl_send_events_vm_stopped(d->id, events); +return; +} old_pending = __sync_fetch_and_or(&d->ram->int_pending, le_events); if ((old_pending & le_events) == le_events) { return; diff --git a/trace-events b/trace-events index 45c6bc1..c22665f 100644 --- a/trace-events +++ b/trace-events @@ -799,6 +799,7 @@ qxl_spice_reset_memslots(int qid) "%d" qxl_spice_update_area(int qid, uint32_t surface_id, uint32_t left, uint32_t right, uint32_t top, uint32_t bottom) "%d sid=%d [%d,%d,%d,%d]" qxl_spice_update_area_rest(int qid, uint32_t num_dirty_rects, uint32_t clear_dirty_region) "%d #d=%d clear=%d" qxl_surfaces_dirty(int qid, int surface, int offset, int size) "%d surface=%d offset=%d size=%d" +qxl_send_events_vm_stopped(int qid, uint32_t events) "%d %d" # hw/qxl-render.c qxl_render_blit_guest_primary_initialized(void) "" -- 1.7.10.4
[Qemu-devel] [PATCH 17/60] net: notify iothread after flushing queue
From: Paolo Bonzini virtio-net has code to flush the queue and notify the iothread whenever new receive buffers are added by the guest. That is fine, and indeed we need to do the same in all other drivers. However, notifying the iothread should be work for the network subsystem. And since we are at it we can add a little smartness: if some of the queued packets already could not be delivered, there is no need to notify the iothread. Reported-by: Luigi Rizzo Cc: Stefan Hajnoczi Cc: Jan Kiszka Signed-off-by: Paolo Bonzini Reviewed-by: Amos Kong Signed-off-by: Stefan Hajnoczi (cherry picked from commit 987a9b483567b1a47a379255e886a77d57ea) Conflicts (backport): net.c net/queue.h Signed-off-by: Michael Tokarev --- hw/virtio-net.c |4 net.c |7 ++- net/queue.c |5 +++-- net/queue.h |2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 3f190d4..0974945 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -447,10 +447,6 @@ static void virtio_net_handle_rx(VirtIODevice *vdev, VirtQueue *vq) VirtIONet *n = to_virtio_net(vdev); qemu_flush_queued_packets(&n->nic->nc); - -/* We now have RX buffers, signal to the IO thread to break out of the - * select to re-poll the tap file descriptor */ -qemu_notify_event(); } static int virtio_net_can_receive(VLANClientState *nc) diff --git a/net.c b/net.c index 1922d8a..fa846ae 100644 --- a/net.c +++ b/net.c @@ -491,7 +491,12 @@ void qemu_flush_queued_packets(VLANClientState *vc) queue = vc->send_queue; } -qemu_net_queue_flush(queue); +if (qemu_net_queue_flush(queue)) { +/* We emptied the queue successfully, signal to the IO thread to repoll + * the file descriptor (for tap, for example). + */ +qemu_notify_event(); +} } static ssize_t qemu_send_packet_async_with_flags(VLANClientState *sender, diff --git a/net/queue.c b/net/queue.c index 1ab5247..395cbd2 100644 --- a/net/queue.c +++ b/net/queue.c @@ -232,7 +232,7 @@ void qemu_net_queue_purge(NetQueue *queue, VLANClientState *from) } } -void qemu_net_queue_flush(NetQueue *queue) +bool qemu_net_queue_flush(NetQueue *queue) { while (!QTAILQ_EMPTY(&queue->packets)) { NetPacket *packet; @@ -248,7 +248,7 @@ void qemu_net_queue_flush(NetQueue *queue) packet->size); if (ret == 0) { QTAILQ_INSERT_HEAD(&queue->packets, packet, entry); -break; +return false; } if (packet->sent_cb) { @@ -257,4 +257,5 @@ void qemu_net_queue_flush(NetQueue *queue) g_free(packet); } +return true; } diff --git a/net/queue.h b/net/queue.h index a31958e..4bf6d3c 100644 --- a/net/queue.h +++ b/net/queue.h @@ -66,6 +66,6 @@ ssize_t qemu_net_queue_send_iov(NetQueue *queue, NetPacketSent *sent_cb); void qemu_net_queue_purge(NetQueue *queue, VLANClientState *from); -void qemu_net_queue_flush(NetQueue *queue); +bool qemu_net_queue_flush(NetQueue *queue); #endif /* QEMU_NET_QUEUE_H */ -- 1.7.10.4
[Qemu-devel] [PATCH 52/60] uhci: Don't queue up packets after one with the SPD flag set
From: Hans de Goede Don't queue up packets after a packet with the SPD (short packet detect) flag set. Since we won't know if the packet will actually be short until it has completed, and if it is short we should stop the queue. This fixes a miniature photoframe emulating a USB cdrom with the windows software for it not working. Signed-off-by: Hans de Goede Signed-off-by: Gerd Hoffmann (cherry picked from commit 72a04d0c178f01908d74539230d9de64ffc6da19) Signed-off-by: Michael Tokarev --- hw/usb/hcd-uhci.c |5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c index a8bb164..766e7ad 100644 --- a/hw/usb/hcd-uhci.c +++ b/hw/usb/hcd-uhci.c @@ -986,6 +986,9 @@ static void uhci_fill_queue(UHCIState *s, UHCI_TD *td) } assert(ret == TD_RESULT_ASYNC_START); assert(int_mask == 0); +if (ptd.ctrl & TD_CTRL_SPD) { +break; +} plink = ptd.link; } } @@ -1083,7 +1086,7 @@ static void uhci_process_frame(UHCIState *s) case TD_RESULT_ASYNC_START: trace_usb_uhci_td_async(curr_qh & ~0xf, link & ~0xf); -if (is_valid(td.link)) { +if (is_valid(td.link) && !(td.ctrl & TD_CTRL_SPD)) { uhci_fill_queue(s, &td); } link = curr_qh ? qh.link : td.link; -- 1.7.10.4
[Qemu-devel] [PATCH 46/60] e1000: Discard oversized packets based on SBP|LPE
From: Michael Contreras Discard packets longer than 16384 when !SBP to match the hardware behavior. Signed-off-by: Michael Contreras Signed-off-by: Stefan Hajnoczi (cherry picked from commit 2c0331f4f7d241995452b99afaf0aab00493334a) Signed-off-by: Michael Tokarev --- hw/e1000.c |7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/e1000.c b/hw/e1000.c index e22ba3d..87c798b 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -61,6 +61,8 @@ static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL); /* this is the size past which hardware will drop packets when setting LPE=0 */ #define MAXIMUM_ETHERNET_VLAN_SIZE 1522 +/* this is the size past which hardware will drop packets when setting LPE=1 */ +#define MAXIMUM_ETHERNET_LPE_SIZE 16384 /* * HW models: @@ -799,8 +801,9 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf, size_t size) } /* Discard oversized packets if !LPE and !SBP. */ -if (size > MAXIMUM_ETHERNET_VLAN_SIZE -&& !(s->mac_reg[RCTL] & E1000_RCTL_LPE) +if ((size > MAXIMUM_ETHERNET_LPE_SIZE || +(size > MAXIMUM_ETHERNET_VLAN_SIZE +&& !(s->mac_reg[RCTL] & E1000_RCTL_LPE))) && !(s->mac_reg[RCTL] & E1000_RCTL_SBP)) { return size; } -- 1.7.10.4
[Qemu-devel] [PATCH 45/60] e1000: Discard packets that are too long if !SBP and !LPE
From: Michael Contreras The e1000_receive function for the e1000 needs to discard packets longer than 1522 bytes if the SBP and LPE flags are disabled. The linux driver assumes this behavior and allocates memory based on this assumption. Signed-off-by: Michael Contreras Signed-off-by: Anthony Liguori (cherry picked from commit b0d9ffcd0251161c7c92f94804dcf599dfa3edeb) Signed-off-by: Michael Tokarev --- hw/e1000.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/hw/e1000.c b/hw/e1000.c index d3a15e5..e22ba3d 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -59,6 +59,9 @@ static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL); #define PNPMMIO_SIZE 0x2 #define MIN_BUF_SIZE 60 /* Min. octets in an ethernet frame sans FCS */ +/* this is the size past which hardware will drop packets when setting LPE=0 */ +#define MAXIMUM_ETHERNET_VLAN_SIZE 1522 + /* * HW models: * E1000_DEV_ID_82540EM works with Windows and Linux @@ -795,6 +798,13 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf, size_t size) size = sizeof(min_buf); } +/* Discard oversized packets if !LPE and !SBP. */ +if (size > MAXIMUM_ETHERNET_VLAN_SIZE +&& !(s->mac_reg[RCTL] & E1000_RCTL_LPE) +&& !(s->mac_reg[RCTL] & E1000_RCTL_SBP)) { +return size; +} + if (!receive_filter(s, buf, size)) return size; -- 1.7.10.4
Re: [Qemu-devel] [PATCH 10/60] pcie_aer: clear cmask for Advanced Error Interrupt Message Number
On Mon, Feb 04, 2013 at 02:40:20PM +0400, Michael Tokarev wrote: > From: Jason Baron > > The Advanced Error Interrupt Message Number (bits 31:27 of the Root > Error Status Register) is updated when the number of msi messages assigned to > a > device changes. Migration of windows 7 on q35 chipset failed because the check > in get_pci_config_device() fails due to cmask being set on these bits. Its > valid > to update these bits and we must restore this state across migration. > > Signed-off-by: Jason Baron > Signed-off-by: Michael S. Tsirkin > (cherry picked from commit 0e180d9c8a7429c55d23d2e7855f1e490a063aaa) > > Signed-off-by: Michael Tokarev q35 isn't part of 1.1 so why do we want this patch there? > --- > hw/pcie_aer.c |5 + > 1 file changed, 5 insertions(+) > > diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c > index 3b6981c..b04c164 100644 > --- a/hw/pcie_aer.c > +++ b/hw/pcie_aer.c > @@ -738,6 +738,11 @@ void pcie_aer_root_init(PCIDevice *dev) > PCI_ERR_ROOT_CMD_EN_MASK); > pci_set_long(dev->w1cmask + pos + PCI_ERR_ROOT_STATUS, > PCI_ERR_ROOT_STATUS_REPORT_MASK); > +/* PCI_ERR_ROOT_IRQ is RO but devices change it using a > + * device-specific method. > + */ > +pci_set_long(dev->cmask + pos + PCI_ERR_ROOT_STATUS, > + ~PCI_ERR_ROOT_IRQ); > } > > void pcie_aer_root_reset(PCIDevice *dev) > -- > 1.7.10.4
[Qemu-devel] [PATCH 38/60] xhci: fix usb name in caps
From: Gerd Hoffmann Used to be "UTB" not "USB". Signed-off-by: Gerd Hoffmann (cherry picked from commit 0ebfb144e8ad3f2da436d630fdcc5aa9ab646341) --- hw/usb/hcd-xhci.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 5cf1a64..10be478 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -2372,7 +2372,7 @@ static uint32_t xhci_cap_read(XHCIState *xhci, uint32_t reg) return 0x0202; /* USB 2.0 */ #endif case 0x24: /* Supported Protocol:04 */ -return 0x20425455; /* "USB " */ +return 0x20425355; /* "USB " */ case 0x28: /* Supported Protocol:08 */ return 0x0001 | (USB2_PORTS<<8); case 0x2c: /* Supported Protocol:0c */ @@ -2381,7 +2381,7 @@ static uint32_t xhci_cap_read(XHCIState *xhci, uint32_t reg) case 0x30: /* Supported Protocol:00 */ return 0x0302; /* USB 3.0 */ case 0x34: /* Supported Protocol:04 */ -return 0x20425455; /* "USB " */ +return 0x20425355; /* "USB " */ case 0x38: /* Supported Protocol:08 */ return 0x | (USB2_PORTS+1) | (USB3_PORTS<<8); case 0x3c: /* Supported Protocol:0c */ -- 1.7.10.4
[Qemu-devel] [PATCH 58/60] qxl: save qemu_create_displaysurface_from result
From: Gerd Hoffmann Spotted by Coverity. https://bugzilla.redhat.com/show_bug.cgi?id=885644 Cc: qemu-sta...@nongnu.org Reported-by: Markus Armbruster Signed-off-by: Gerd Hoffmann (cherry picked from commit 2f464b5a32b414adb545acc6d94b5c35c7d258ba) Signed-off-by: Michael Tokarev --- hw/qxl-render.c | 11 ++- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/hw/qxl-render.c b/hw/qxl-render.c index b66c168..e7d41ec 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -113,11 +113,12 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl) qxl->guest_primary.bits_pp); if (qxl->guest_primary.qxl_stride > 0) { qemu_free_displaysurface(vga->ds); -qemu_create_displaysurface_from(qxl->guest_primary.surface.width, -qxl->guest_primary.surface.height, -qxl->guest_primary.bits_pp, -qxl->guest_primary.abs_stride, -qxl->guest_primary.data); +vga->ds->surface = qemu_create_displaysurface_from +(qxl->guest_primary.surface.width, + qxl->guest_primary.surface.height, + qxl->guest_primary.bits_pp, + qxl->guest_primary.abs_stride, + qxl->guest_primary.data); } else { qemu_resize_displaysurface(vga->ds, qxl->guest_primary.surface.width, -- 1.7.10.4
[Qemu-devel] [PATCH 27/60] x86: Fixed incorrect segment base address addition in 64-bits mode
From: Vitaly Chipounov According to the Intel manual "Intel® 64 and IA-32 Architectures Software Developer’s Manual Volume 3", "3.4.4 Segment Loading Instructions in IA-32e Mode": "When in compatibility mode, FS and GS overrides operate as defined by 32-bit mode behavior regardless of the value loaded into the upper 32 linear-address bits of the hidden descriptor register base field. Compatibility mode ignores the upper 32 bits when calculating an effective address." However, the code misses the 64-bit mode case, where an instruction with address and segment size override would be translated incorrectly. For example, inc dword ptr gs:260h[ebx*4] gets incorrectly translated to: (uint32_t)(gs.base + ebx * 4 + 0x260) instead of gs.base + (uint32_t)(ebx * 4 + 0x260) Signed-off-by: Vitaly Chipounov Reviewed-by: Max Filippov Signed-off-by: Blue Swirl (cherry picked from commit 7162ab21fe8e82f924002951cd8e87f69358f8b5) Signed-off-by: Michael Tokarev --- target-i386/translate.c | 43 +-- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/target-i386/translate.c b/target-i386/translate.c index c792e7a..7c69ec0 100644 --- a/target-i386/translate.c +++ b/target-i386/translate.c @@ -456,12 +456,19 @@ static inline void gen_op_movl_A0_seg(int reg) tcg_gen_ld32u_tl(cpu_A0, cpu_env, offsetof(CPUX86State, segs[reg].base) + REG_L_OFFSET); } -static inline void gen_op_addl_A0_seg(int reg) +static inline void gen_op_addl_A0_seg(DisasContext *s, int reg) { tcg_gen_ld_tl(cpu_tmp0, cpu_env, offsetof(CPUX86State, segs[reg].base)); -tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); #ifdef TARGET_X86_64 -tcg_gen_andi_tl(cpu_A0, cpu_A0, 0x); +if (CODE64(s)) { +tcg_gen_andi_tl(cpu_A0, cpu_A0, 0x); +tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); +} else { +tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); +tcg_gen_andi_tl(cpu_A0, cpu_A0, 0x); +} +#else +tcg_gen_add_tl(cpu_A0, cpu_A0, cpu_tmp0); #endif } @@ -617,7 +624,7 @@ static inline void gen_string_movl_A0_ESI(DisasContext *s) override = R_DS; gen_op_movl_A0_reg(R_ESI); gen_op_andl_A0_(); -gen_op_addl_A0_seg(override); +gen_op_addl_A0_seg(s, override); } } @@ -638,7 +645,7 @@ static inline void gen_string_movl_A0_EDI(DisasContext *s) } else { gen_op_movl_A0_reg(R_EDI); gen_op_andl_A0_(); -gen_op_addl_A0_seg(R_ES); +gen_op_addl_A0_seg(s, R_ES); } } @@ -2063,7 +2070,7 @@ static void gen_lea_modrm(DisasContext *s, int modrm, int *reg_ptr, int *offset_ } else #endif { -gen_op_addl_A0_seg(override); +gen_op_addl_A0_seg(s, override); } } } else { @@ -2130,7 +2137,7 @@ static void gen_lea_modrm(DisasContext *s, int modrm, int *reg_ptr, int *offset_ else override = R_DS; } -gen_op_addl_A0_seg(override); +gen_op_addl_A0_seg(s, override); } } @@ -2207,7 +2214,7 @@ static void gen_add_A0_ds_seg(DisasContext *s) } else #endif { -gen_op_addl_A0_seg(override); +gen_op_addl_A0_seg(s, override); } } } @@ -2460,12 +2467,12 @@ static void gen_push_T0(DisasContext *s) if (s->ss32) { if (s->addseg) { tcg_gen_mov_tl(cpu_T[1], cpu_A0); -gen_op_addl_A0_seg(R_SS); +gen_op_addl_A0_seg(s, R_SS); } } else { gen_op_andl_A0_(); tcg_gen_mov_tl(cpu_T[1], cpu_A0); -gen_op_addl_A0_seg(R_SS); +gen_op_addl_A0_seg(s, R_SS); } gen_op_st_T0_A0(s->dflag + 1 + s->mem_index); if (s->ss32 && !s->addseg) @@ -2500,11 +2507,11 @@ static void gen_push_T1(DisasContext *s) gen_op_addl_A0_im(-4); if (s->ss32) { if (s->addseg) { -gen_op_addl_A0_seg(R_SS); +gen_op_addl_A0_seg(s, R_SS); } } else { gen_op_andl_A0_(); -gen_op_addl_A0_seg(R_SS); +gen_op_addl_A0_seg(s, R_SS); } gen_op_st_T1_A0(s->dflag + 1 + s->mem_index); @@ -2528,10 +2535,10 @@ static void gen_pop_T0(DisasContext *s) gen_op_movl_A0_reg(R_ESP); if (s->ss32) { if (s->addseg) -gen_op_addl_A0_seg(R_SS); +gen_op_addl_A0_seg(s, R_SS); } else { gen_op_andl_A0_(); -gen_op_addl_A0_seg(R_SS); +gen_op_addl_A0_seg(s, R_SS); } gen_op_ld_T0_A0(s->dflag + 1 + s->mem_index); } @@ -2556,7 +2563,7 @@ static void gen_stack_A0(DisasContext *s) gen_op_andl_A0_(); tcg_gen_mov_tl(cpu_T[1], cpu_A0); if (s->addseg) -gen_op_a
[Qemu-devel] [PATCH 50/60] slirp: Don't crash on packets from 0.0.0.0/8.
From: Nickolai Zeldovich LWIP can generate packets with a source of 0.0.0.0, which triggers an assertion failure in arp_table_add(). Instead of crashing, simply return to avoid adding an invalid ARP table entry. Signed-off-by: Nickolai Zeldovich Signed-off-by: Jan Kiszka (cherry picked from commit 1a89b60885ccc2abf7cc50275fcee70d0347425e) Signed-off-by: Michael Tokarev --- slirp/arp_table.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/slirp/arp_table.c b/slirp/arp_table.c index 5d7b8ac..bf698c1 100644 --- a/slirp/arp_table.c +++ b/slirp/arp_table.c @@ -38,7 +38,9 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN]) ethaddr[3], ethaddr[4], ethaddr[5])); /* Check 0.0.0.0/8 invalid source-only addresses */ -assert((ip_addr & htonl(~(0xf << 28))) != 0); +if ((ip_addr & htonl(~(0xf << 28))) == 0) { +return; +} if (ip_addr == 0x || ip_addr == broadcast_addr) { /* Do not register broadcast addresses */ -- 1.7.10.4
[Qemu-devel] [PATCH 12/60] intel_hda: do not call msi_reset when only device state needs resetting
Commit 8e729e3b521d9 "intel-hda: Fix reset of MSI function" (applied to 1.1.1 as 0ec39075710) added a call to msi_reset() into intel_hda_reset() function. But this function is called not only from PCI bus reset method, but also from device init method (intel_hda_set_g_ctl()), and there, we should not reset msi state. For this, split intel_hda_reset() into two halves, one common part with device reset, and one with msi reset, intel_hda_reset_msi(), which also calls the common part, for the bus method. This is only needed for 1.1.x series, since in 1.2+, MSI reset is called in proper places by the PCI code already. Signed-off-by: Michael Tokarev Cc: Jan Kiszka Cc: Michael S. Tsirkin Cc: 688...@bugs.debian.org --- hw/intel-hda.c | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/hw/intel-hda.c b/hw/intel-hda.c index e38861e..da61323 100644 --- a/hw/intel-hda.c +++ b/hw/intel-hda.c @@ -1107,9 +1107,6 @@ static void intel_hda_reset(DeviceState *dev) DeviceState *qdev; HDACodecDevice *cdev; -if (d->msi) { -msi_reset(&d->pci); -} intel_hda_regs_reset(d); d->wall_base_ns = qemu_get_clock_ns(vm_clock); @@ -1122,6 +1119,15 @@ static void intel_hda_reset(DeviceState *dev) intel_hda_update_irq(d); } +static void intel_hda_reset_msi(DeviceState *dev) +{ +IntelHDAState *d = DO_UPCAST(IntelHDAState, pci.qdev, dev); +if (d->msi) { +msi_reset(&d->pci); +} +intel_hda_reset(dev); +} + static int intel_hda_init(PCIDevice *pci) { IntelHDAState *d = DO_UPCAST(IntelHDAState, pci, pci); @@ -1261,7 +1267,7 @@ static void intel_hda_class_init(ObjectClass *klass, void *data) k->revision = 1; k->class_id = PCI_CLASS_MULTIMEDIA_HD_AUDIO; dc->desc = "Intel HD Audio Controller"; -dc->reset = intel_hda_reset; +dc->reset = intel_hda_reset_msi; dc->vmsd = &vmstate_intel_hda; dc->props = intel_hda_properties; } -- 1.7.10.4
[Qemu-devel] [PATCH 43/60] qcow2: Fix avail_sectors in cluster allocation code
From: Kevin Wolf avail_sectors should really be the number of sectors from the start of the allocation, not from the start of the write request. We're lucky enough that this mistake didn't cause any real bug. avail_sectors is only used in the intialiser of QCowL2Meta: .nb_available = MIN(requested_sectors, avail_sectors), m->nb_available in turn is only used for COW at the end of the allocation. A COW occurs only if the request wasn't cluster aligned, which in turn would imply that requested_sectors was less than avail_sectors (both in the original and in the fixed version). In this case avail_sectors is ignored and therefore the mistake doesn't cause any misbehaviour. Signed-off-by: Kevin Wolf (cherry picked from commit b7ab0fea37c15ca9e249c42c46f5c48fd1a0943c) Signed-off-by: Michael Tokarev --- block/qcow2-cluster.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index c173fcd..58e7e24 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -949,8 +949,16 @@ again: /* save info needed for meta data update */ if (nb_clusters > 0) { +/* + * requested_sectors: Number of sectors from the start of the first + * newly allocated cluster to the end of the (possibly shortened + * before) write request. + * + * avail_sectors: Number of sectors from the start of the first + * newly allocated to the end of the last newly allocated cluster. + */ int requested_sectors = n_end - keep_clusters * s->cluster_sectors; -int avail_sectors = (keep_clusters + nb_clusters) +int avail_sectors = nb_clusters << (s->cluster_bits - BDRV_SECTOR_BITS); *m = (QCowL2Meta) { -- 1.7.10.4
[Qemu-devel] [PATCH 44/60] qcow2: Fix refcount table size calculation
From: Kevin Wolf A missing factor for the refcount table entry size in the calculation could mean that too little memory was allocated for the in-memory representation of the table, resulting in a buffer overflow. Signed-off-by: Kevin Wolf Reviewed-by: Michael Tokarev Tested-by: Michael Tokarev (cherry picked from commit a3548077062dd9dc2701ebffd931ba6eaef40bec) Signed-off-by: Michael Tokarev --- block/qcow2-refcount.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 443c021..645c7c5 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -301,7 +301,8 @@ static int alloc_refcount_block(BlockDriverState *bs, uint64_t last_table_size; uint64_t blocks_clusters; do { -uint64_t table_clusters = size_to_clusters(s, table_size); +uint64_t table_clusters = +size_to_clusters(s, table_size * sizeof(uint64_t)); blocks_clusters = 1 + ((table_clusters + refcount_block_clusters - 1) / refcount_block_clusters); -- 1.7.10.4
[Qemu-devel] [PATCH 14/60] fpu/softfloat.c: Return correctly signed values from uint64_to_float32
From: Peter Maydell The uint64_to_float32() conversion function was incorrectly always returning numbers with the sign bit set (ie negative numbers). Correct this so we return positive numbers instead. Signed-off-by: Peter Maydell Signed-off-by: Aurelien Jarno (cherry picked from commit e744c06fca438dc08271e626034e632a270c91c8) Signed-off-by: Michael Tokarev --- fpu/softfloat.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fpu/softfloat.c b/fpu/softfloat.c index b29256a..91497e8 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -1238,7 +1238,7 @@ float32 uint64_to_float32( uint64 a STATUS_PARAM ) if ( a == 0 ) return float32_zero; shiftCount = countLeadingZeros64( a ) - 40; if ( 0 <= shiftCount ) { -return packFloat32( 1 > 0, 0x95 - shiftCount, a< 0, 0x9C - shiftCount, a STATUS_VAR ); +return roundAndPackFloat32(0, 0x9C - shiftCount, a STATUS_VAR); } } -- 1.7.10.4
[Qemu-devel] [PATCH 39/60] m68k: Return semihosting errno values correctly
From: Meador Inge Fixing a simple typo, s/errno/err/, that caused the error status from GDB semihosted system calls to be returned incorrectly. Signed-off-by: Meador Inge Reviewed-by: Andreas Färber Signed-off-by: Peter Maydell Signed-off-by: Blue Swirl (cherry picked from commit aed91c1bff5e568c7b0fbd0e1e7e2f9e62409e73) Signed-off-by: Michael Tokarev --- m68k-semi.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/m68k-semi.c b/m68k-semi.c index 3bb30cd..fed44ea 100644 --- a/m68k-semi.c +++ b/m68k-semi.c @@ -150,7 +150,7 @@ static void m68k_semi_cb(CPUM68KState *env, target_ulong ret, target_ulong err) } /* FIXME - handle put_user() failure */ put_user_u32(ret, args); -put_user_u32(errno, args + 4); +put_user_u32(err, args + 4); } #define ARG(n) \ -- 1.7.10.4
[Qemu-devel] [PATCH 59/60] target-xtensa: fix ITLB/DTLB page protection flags
From: Max Filippov With MMU option xtensa architecture has two TLBs: ITLB and DTLB. ITLB is only used for code access, DTLB is only for data. However TLB entries in both TLBs have attribute field controlling write and exec access. These bits need to be properly masked off depending on TLB type before being used as tlb_set_page prot argument. Otherwise the following happens: (1) ITLB entry for some PFN gets invalidated (2) DTLB entry for the same PFN gets updated, attributes allow code execution (3) code at the page with that PFN is executed (possible due to step 2), entry for the TB is written into the jump cache (4) QEMU TLB entry for the PFN gets replaced with an entry for some other PFN (5) code in the TB from step 3 is executed (possible due to jump cache) and it accesses data, for which there's no DTLB entry, causing DTLB miss exception (6) re-translation of the TB from step 5 is attempted, but there's no QEMU TLB entry nor xtensa ITLB entry for that PFN, which causes ITLB miss exception at the TB start address (7) ITLB miss exception is handled by the guest, but execution is resumed from the beginning of the faulting TB (the point where ITLB miss occured), not from the point where DTLB miss occured, which is wrong. With that fix the above scenario causes ITLB miss exception (that used to be step 7) at step 3, right at the beginning of the TB. Signed-off-by: Max Filippov Cc: qemu-sta...@nongnu.org Signed-off-by: Blue Swirl (cherry picked from commit 659f807c0a700317a7a0fae7a6e6ebfe68bfbbc4) Signed-off-by: Michael Tokarev --- target-xtensa/helper.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c index 8ebef72..c9d6f38 100644 --- a/target-xtensa/helper.c +++ b/target-xtensa/helper.c @@ -497,7 +497,8 @@ static int get_physical_addr_mmu(CPUXtensaState *env, bool update_tlb, INST_FETCH_PRIVILEGE_CAUSE; } -*access = mmu_attr_to_access(entry->attr); +*access = mmu_attr_to_access(entry->attr) & +~(dtlb ? PAGE_EXEC : PAGE_READ | PAGE_WRITE); if (!is_access_granted(*access, is_write)) { return dtlb ? (is_write ? -- 1.7.10.4
Re: [Qemu-devel] [PATCH 10/60] pcie_aer: clear cmask for Advanced Error Interrupt Message Number
04.02.2013 14:52, Michael S. Tsirkin wrote: On Mon, Feb 04, 2013 at 02:40:20PM +0400, Michael Tokarev wrote: From: Jason Baron The Advanced Error Interrupt Message Number (bits 31:27 of the Root Error Status Register) is updated when the number of msi messages assigned to a device changes. Migration of windows 7 on q35 chipset failed because the check in get_pci_config_device() fails due to cmask being set on these bits. Its valid to update these bits and we must restore this state across migration. Signed-off-by: Jason Baron Signed-off-by: Michael S. Tsirkin (cherry picked from commit 0e180d9c8a7429c55d23d2e7855f1e490a063aaa) Signed-off-by: Michael Tokarev q35 isn't part of 1.1 so why do we want this patch there? Looks like we don't, I dropped this patch now. Thank you for your feedback! /mjt
Re: [Qemu-devel] [PATCH qom-cpu-next v3 2/4] target-i386: Split command line parsing out of cpu_x86_register()
On Sat, 2 Feb 2013 01:37:06 +0100 Andreas Färber wrote: > In order to instantiate a CPU subtype we will need to know which type, > so move the cpu_model splitting into cpu_x86_init(). > > Parameters need to be set on the X86CPU instance, so move > cpu_x86_parse_featurestr() into cpu_x86_init() as well. > > This leaves cpu_x86_register() operating on the model name only. > > Signed-off-by: Andreas Färber > --- > target-i386/cpu.c | 50 +++--- > 1 Datei geändert, 31 Zeilen hinzugefügt(+), 19 Zeilen entfernt(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index e74802b..ee2fd6b 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1516,24 +1516,14 @@ static void filter_features_for_kvm(X86CPU *cpu) > } > #endif > > -static int cpu_x86_register(X86CPU *cpu, const char *cpu_model) > +static int cpu_x86_register(X86CPU *cpu, const char *name) > { > CPUX86State *env = &cpu->env; > x86_def_t def1, *def = &def1; > Error *error = NULL; > -char *name, *features; > -gchar **model_pieces; > > memset(def, 0, sizeof(*def)); > > -model_pieces = g_strsplit(cpu_model, ",", 2); > -if (!model_pieces[0]) { > -error_setg(&error, "Invalid/empty CPU model name"); > -goto out; > -} > -name = model_pieces[0]; > -features = model_pieces[1]; > - > if (cpu_x86_find_by_name(def, name) < 0) { > error_setg(&error, "Unable to find CPU definition: %s", name); > goto out; > @@ -1565,9 +1555,7 @@ static int cpu_x86_register(X86CPU *cpu, const char > *cpu_model) goto out; > } > > -cpu_x86_parse_featurestr(cpu, features, &error); > out: > -g_strfreev(model_pieces); > if (error) { > fprintf(stderr, "%s\n", error_get_pretty(error)); > error_free(error); > @@ -1578,26 +1566,50 @@ out: > > X86CPU *cpu_x86_init(const char *cpu_model) > { > -X86CPU *cpu; > +X86CPU *cpu = NULL; > CPUX86State *env; > +gchar **model_pieces; > +char *name, *features; > Error *error = NULL; > > +model_pieces = g_strsplit(cpu_model, ",", 2); > +if (!model_pieces[0]) { > +error_setg(&error, "Invalid/empty CPU model name"); > +goto error; > +} > +name = model_pieces[0]; > +features = model_pieces[1]; > + > cpu = X86_CPU(object_new(TYPE_X86_CPU)); > env = &cpu->env; > env->cpu_model_str = cpu_model; > > -if (cpu_x86_register(cpu, cpu_model) < 0) { > -object_delete(OBJECT(cpu)); > -return NULL; > +if (cpu_x86_register(cpu, name) < 0) { > +goto error; > +} > + > +cpu_x86_parse_featurestr(cpu, features, &error); > +if (error) { > +goto error; > } > > object_property_set_bool(OBJECT(cpu), true, "realized", &error); > if (error) { > +goto error; > +} > +g_strfreev(model_pieces); > +return cpu; > + > +error: Could you use common exit path like it's done in cpu_x86_register()? That allows to avoid cleanup code duplication on exit. > +g_strfreev(model_pieces); > +if (error) { > +fprintf(stderr, "%s\n", error_get_pretty(error)); > error_free(error); > +} > +if (cpu != NULL) { > object_delete(OBJECT(cpu)); > -return NULL; > } > -return cpu; > +return NULL; > } > > #if !defined(CONFIG_USER_ONLY)
[Qemu-devel] [PATCH 55/60] arm_boot: Change initrd load address to "halfway through RAM"
From: Peter Maydell To avoid continually having to bump the initrd load address to account for larger kernel images, put the initrd halfway through RAM. This allows large kernels on new boards with lots of RAM to work OK, without breaking existing usecases for boards with only 32MB of RAM. Note that this change fixes in passing a bug where we were passing an overly large max_size to load_image_targphys() for the initrd, which meant that we wouldn't correctly refuse to load an enormous initrd that didn't actually fit into RAM. Signed-off-by: Peter Maydell Reviewed-by: Aurelien Jarno Reviewed-by: Igor Mitsyanko Tested-by: Cole Robinson Signed-off-by: Aurelien Jarno (cherry picked from commit fc53b7d4b7fe409acae7d8d55a868eb5c696d71c) Conflicts (target_phys_addr_t vs hwaddr change): hw/arm-misc.h hw/arm_boot.c Signed-off-by: Michael Tokarev --- hw/arm-misc.h |1 + hw/arm_boot.c | 40 +--- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/hw/arm-misc.h b/hw/arm-misc.h index 2f46e21..f0bd5a9 100644 --- a/hw/arm-misc.h +++ b/hw/arm-misc.h @@ -57,6 +57,7 @@ struct arm_boot_info { /* Used internally by arm_boot.c */ int is_linux; target_phys_addr_t initrd_size; +target_phys_addr_t initrd_start; target_phys_addr_t entry; }; void arm_load_kernel(CPUARMState *env, struct arm_boot_info *info); diff --git a/hw/arm_boot.c b/hw/arm_boot.c index 05701b2..0a9b122 100644 --- a/hw/arm_boot.c +++ b/hw/arm_boot.c @@ -18,7 +18,6 @@ #define KERNEL_ARGS_ADDR 0x100 #define KERNEL_LOAD_ADDR 0x0001 -#define INITRD_LOAD_ADDR 0x00d0 /* The worlds second smallest bootloader. Set r0-r2, then jump to kernel. */ static uint32_t bootloader[] = { @@ -107,7 +106,7 @@ static void set_kernel_args(const struct arm_boot_info *info) /* ATAG_INITRD2 */ WRITE_WORD(p, 4); WRITE_WORD(p, 0x54420005); -WRITE_WORD(p, info->loader_start + INITRD_LOAD_ADDR); +WRITE_WORD(p, info->initrd_start); WRITE_WORD(p, initrd_size); } if (info->kernel_cmdline && *info->kernel_cmdline) { @@ -183,10 +182,11 @@ static void set_kernel_args_old(const struct arm_boot_info *info) /* pages_in_vram */ WRITE_WORD(p, 0); /* initrd_start */ -if (initrd_size) -WRITE_WORD(p, info->loader_start + INITRD_LOAD_ADDR); -else +if (initrd_size) { +WRITE_WORD(p, info->initrd_start); +} else { WRITE_WORD(p, 0); +} /* initrd_size */ WRITE_WORD(p, initrd_size); /* rd_start */ @@ -248,14 +248,13 @@ static int load_dtb(target_phys_addr_t addr, const struct arm_boot_info *binfo) if (binfo->initrd_size) { rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-start", -binfo->loader_start + INITRD_LOAD_ADDR); +binfo->initrd_start); if (rc < 0) { fprintf(stderr, "couldn't set /chosen/linux,initrd-start\n"); } rc = qemu_devtree_setprop_cell(fdt, "/chosen", "linux,initrd-end", -binfo->loader_start + INITRD_LOAD_ADDR + -binfo->initrd_size); +binfo->initrd_start + binfo->initrd_size); if (rc < 0) { fprintf(stderr, "couldn't set /chosen/linux,initrd-end\n"); } @@ -340,6 +339,19 @@ void arm_load_kernel(CPUARMState *env, struct arm_boot_info *info) big_endian = 0; #endif +/* We want to put the initrd far enough into RAM that when the + * kernel is uncompressed it will not clobber the initrd. However + * on boards without much RAM we must ensure that we still leave + * enough room for a decent sized initrd, and on boards with large + * amounts of RAM we must avoid the initrd being so far up in RAM + * that it is outside lowmem and inaccessible to the kernel. + * So for boards with less than 256MB of RAM we put the initrd + * halfway into RAM, and for boards with 256MB of RAM or more we put + * the initrd at 128MB. + */ +info->initrd_start = info->loader_start + +MIN(info->ram_size / 2, 128 * 1024 * 1024); + /* Assume that raw images are linux kernels, and ELF images are not. */ kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry, NULL, NULL, big_endian, ELF_MACHINE, 1); @@ -363,10 +375,9 @@ void arm_load_kernel(CPUARMState *env, struct arm_boot_info *info) if (is_linux) { if (info->initrd_filename) { initrd_size = load_image_targphys(info->initrd_filename, - info->loader_start - + INITRD_LOAD_ADDR, - info->ram_size - - INITRD_LOAD_ADDR); + info->initrd_start, +
Re: [Qemu-devel] [PATCH 10/60] pcie_aer: clear cmask for Advanced Error Interrupt Message Number
04.02.2013 14:56, Michael Tokarev wrote: 04.02.2013 14:52, Michael S. Tsirkin wrote: On Mon, Feb 04, 2013 at 02:40:20PM +0400, Michael Tokarev wrote: From: Jason Baron The Advanced Error Interrupt Message Number (bits 31:27 of the Root Error Status Register) is updated when the number of msi messages assigned to a device changes. Migration of windows 7 on q35 chipset failed because the check in get_pci_config_device() fails due to cmask being set on these bits. Its valid to update these bits and we must restore this state across migration. Signed-off-by: Jason Baron Signed-off-by: Michael S. Tsirkin (cherry picked from commit 0e180d9c8a7429c55d23d2e7855f1e490a063aaa) Signed-off-by: Michael Tokarev q35 isn't part of 1.1 so why do we want this patch there? Looks like we don't, I dropped this patch now. Actually... isn't it the same when you use ahci device? Thanks, /mjt
Re: [Qemu-devel] [PATCH V20 1/8] Support for TPM command line options
On Fri, 01 Feb 2013 10:33:01 -0500 Corey Bryant wrote: > > +## > > +# @TPMInfo: > > +# > > +# Information about the TPM > > +# > > +# @model: The TPM frontend model, i.e., tpm-tis > > +# > > +# @id: The ID of the TPM > > +# > > +# @type: The type of TPM backend, i.e., passthrough > > +# > > +# @path: #optional Path to the TPM backend device > > +# > > +# @cancel_path: #optional Path to TPM backend device's cancel sysfs entry > > +# > > +# Since: 1.5.0 > > +## > > +{ 'type': 'TPMInfo', > > + 'data': {'model': 'str', 'id': 'str', 'type': 'str', '*path': 'str', > > + '*cancel_path': 'str' } } > > + > > It might be preferred that you break the monitor support into it's own > patch, and logically it would make more sense to introduce these backend > specific members after the backend patch. But those are just nit comments. > > More importantly, I have a question (probably for Luiz) regarding future > modification of the monitor command. In the future, if a new vTPM > backend is introduced (e.g. an emulated software vTPM), can we add new > optional members to the end of this TPMInfo? For example, could we > modify it to this in the future? Usually, extensions like that are only allowed for query- commands. For non-query commands we prefer adding new commands instead. Btw, we avoid using free strings like 'type' and 'model', please use an enumeration instead. PS: I wonder if it would be possible to extend enumerations and unions though.
[Qemu-devel] [PATCH 06/60] fix doc of using raw values with sendkey
From: Amos Kong (qemu) sendkey a (qemu) sendkey 0x1e (qemu) sendkey #0x1e unknown key: '#0x1e' The last command doesn't work, '#' is not requested before raw values, and the raw value in decimal format is not supported. Signed-off-by: Amos Kong Signed-off-by: Luiz Capitulino (cherry picked from commit 886cc706ce5d4d3d1c296f028ddc2991cfbe3bbe) Signed-off-by: Michael Tokarev --- hmp-commands.hx |6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 18cb415..f4dc4a0 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -512,9 +512,9 @@ STEXI @item sendkey @var{keys} @findex sendkey -Send @var{keys} to the emulator. @var{keys} could be the name of the -key or @code{#} followed by the raw value in either decimal or hexadecimal -format. Use @code{-} to press several keys simultaneously. Example: +Send @var{keys} to the guest. @var{keys} could be the name of the +key or the raw value in hexadecimal format. Use @code{-} to press +several keys simultaneously. Example: @example sendkey ctrl-alt-f1 @end example -- 1.7.10.4
[Qemu-devel] [PATCH 23/60] hw/qxl: qxl_dirty_surfaces: use uintptr_t
From: Alon Levy As suggested by Paolo Bonzini, to avoid possible integer overflow issues. Signed-off-by: Alon Levy Signed-off-by: Gerd Hoffmann (cherry picked from commit c5825ac6c861bfe1a4adfa27517931b56079e298) Signed-off-by: Michael Tokarev --- hw/qxl.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/qxl.c b/hw/qxl.c index 3da3399..b971ac7 100644 --- a/hw/qxl.c +++ b/hw/qxl.c @@ -1603,7 +1603,7 @@ static void qxl_hw_text_update(void *opaque, console_ch_t *chardata) static void qxl_dirty_surfaces(PCIQXLDevice *qxl) { -intptr_t vram_start; +uintptr_t vram_start; int i; if (qxl->mode != QXL_MODE_NATIVE && qxl->mode != QXL_MODE_COMPAT) { @@ -1614,7 +1614,7 @@ static void qxl_dirty_surfaces(PCIQXLDevice *qxl) qxl_set_dirty(&qxl->vga.vram, qxl->shadow_rom.draw_area_offset, qxl->shadow_rom.surface0_area_size); -vram_start = (intptr_t)memory_region_get_ram_ptr(&qxl->vram_bar); +vram_start = (uintptr_t)memory_region_get_ram_ptr(&qxl->vram_bar); /* dirty the off-screen surfaces */ for (i = 0; i < NUM_SURFACES; i++) { -- 1.7.10.4
[Qemu-devel] [PATCH 28/60] Fixes related to processing of qemu's -numa option
From: Chegu Vinod The -numa option to qemu is used to create [fake] numa nodes and expose them to the guest OS instance. There are a couple of issues with the -numa option: a) Max VCPU's that can be specified for a guest while using the qemu's -numa option is 64. Due to a typecasting issue when the number of VCPUs is > 32 the VCPUs don't show up under the specified [fake] numa nodes. b) KVM currently has support for 160VCPUs per guest. The qemu's -numa option has only support for upto 64VCPUs per guest. This patch addresses these two issues. Below are examples of (a) and (b) a) >32 VCPUs are specified with the -numa option: /usr/local/bin/qemu-system-x86_64 \ -enable-kvm \ 71:01:01 \ -net tap,ifname=tap0,script=no,downscript=no \ -vnc :4 ... Upstream qemu : -- QEMU 1.1.50 monitor - type 'help' for more information (qemu) info numa 6 nodes node 0 cpus: 0 1 2 3 4 5 6 7 8 9 32 33 34 35 36 37 38 39 40 41 node 0 size: 131072 MB node 1 cpus: 10 11 12 13 14 15 16 17 18 19 42 43 44 45 46 47 48 49 50 51 node 1 size: 131072 MB node 2 cpus: 20 21 22 23 24 25 26 27 28 29 52 53 54 55 56 57 58 59 node 2 size: 131072 MB node 3 cpus: 30 node 3 size: 131072 MB node 4 cpus: node 4 size: 131072 MB node 5 cpus: 31 node 5 size: 131072 MB With the patch applied : --- QEMU 1.1.50 monitor - type 'help' for more information (qemu) info numa 6 nodes node 0 cpus: 0 1 2 3 4 5 6 7 8 9 node 0 size: 131072 MB node 1 cpus: 10 11 12 13 14 15 16 17 18 19 node 1 size: 131072 MB node 2 cpus: 20 21 22 23 24 25 26 27 28 29 node 2 size: 131072 MB node 3 cpus: 30 31 32 33 34 35 36 37 38 39 node 3 size: 131072 MB node 4 cpus: 40 41 42 43 44 45 46 47 48 49 node 4 size: 131072 MB node 5 cpus: 50 51 52 53 54 55 56 57 58 59 node 5 size: 131072 MB b) >64 VCPUs specified with -numa option: /usr/local/bin/qemu-system-x86_64 \ -enable-kvm \ -cpu Westmere,+rdtscp,+pdpe1gb,+dca,+pdcm,+xtpr,+tm2,+est,+smx,+vmx,+ds_cpl,+monitor,+dtes64,+pclmuldq,+pbe,+tm,+ht,+ss,+acpi,+d-vnc :4 ... Upstream qemu : -- only 63 CPUs in NUMA mode supported. only 64 CPUs in NUMA mode supported. QEMU 1.1.50 monitor - type 'help' for more information (qemu) info numa 8 nodes node 0 cpus: 6 7 8 9 38 39 40 41 70 71 72 73 node 0 size: 65536 MB node 1 cpus: 10 11 12 13 14 15 16 17 18 19 42 43 44 45 46 47 48 49 50 51 74 75 76 77 78 79 node 1 size: 65536 MB node 2 cpus: 20 21 22 23 24 25 26 27 28 29 52 53 54 55 56 57 58 59 60 61 node 2 size: 65536 MB node 3 cpus: 30 62 node 3 size: 65536 MB node 4 cpus: node 4 size: 65536 MB node 5 cpus: node 5 size: 65536 MB node 6 cpus: 31 63 node 6 size: 65536 MB node 7 cpus: 0 1 2 3 4 5 32 33 34 35 36 37 64 65 66 67 68 69 node 7 size: 65536 MB With the patch applied : --- QEMU 1.1.50 monitor - type 'help' for more information (qemu) info numa 8 nodes node 0 cpus: 0 1 2 3 4 5 6 7 8 9 node 0 size: 65536 MB node 1 cpus: 10 11 12 13 14 15 16 17 18 19 node 1 size: 65536 MB node 2 cpus: 20 21 22 23 24 25 26 27 28 29 node 2 size: 65536 MB node 3 cpus: 30 31 32 33 34 35 36 37 38 39 node 3 size: 65536 MB node 4 cpus: 40 41 42 43 44 45 46 47 48 49 node 4 size: 65536 MB node 5 cpus: 50 51 52 53 54 55 56 57 58 59 node 5 size: 65536 MB node 6 cpus: 60 61 62 63 64 65 66 67 68 69 node 6 size: 65536 MB node 7 cpus: 70 71 72 73 74 75 76 77 78 79 Signed-off-by: Chegu Vinod , Jim Hull , Craig Hada Tested-by: Eduardo Habkost Reviewed-by: Eduardo Habkost Signed-off-by: Blue Swirl (cherry picked from commit ee785fed5dd035d4b12142cacec6d3c344426dec) Signed-off-by: Michael Tokarev --- cpus.c |3 ++- hw/pc.c |3 ++- sysemu.h |3 ++- vl.c | 43 +-- 4 files changed, 27 insertions(+), 25 deletions(-) diff --git a/cpus.c b/cpus.c index b182b3d..acccd08 100644 --- a/cpus.c +++ b/cpus.c @@ -36,6 +36,7 @@ #include "cpus.h" #include "qtest.h" #include "main-loop.h" +#include "bitmap.h" #ifndef _WIN32 #include "compatfd.h" @@ -1145,7 +1146,7 @@ void set_numa_modes(void) for (env = first_cpu; env != NULL; env = env->next_cpu) { for (i = 0; i < nb_numa_nodes; i++) { -if (node_cpumask[i] & (1 << env->cpu_index)) { +if (test_bit(env->cpu_index, node_cpumask[i])) { env->numa_node = i; } } diff --git a/hw/pc.c b/hw/pc.c index 6c4b07b..2aa7551 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -48,6 +48,7 @@ #include "memory.h" #include "exec-memory.h" #include "arch_init.h" +#include "bitmap.h" /* output Bochs bios info messages */ //#define DEBUG_BIOS @@ -644,7 +645,7 @@ static void *bochs_bios_init(void) numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes); for (i = 0; i < max_cpus; i++) { for (j = 0; j < nb_numa_nodes; j++) { -if (node_cpumask[j] & (1 << i)) { +if (test_bit(i, node_cpumask[j])) { numa_fw_cfg[i + 1] = cpu_to_le64(j); break; } diff --git a/sysem
[Qemu-devel] [PATCH 08/60] eepro100: Fix network hang when rx buffers run out
From: Bo Yang This is reported by QA. When installing os with pxe, after the initial kernel and initrd are loaded, the procedure tries to copy files from install server to local harddisk, the network becomes stall because of running out of receive descriptor. [Whitespace fixes and removed qemu_notify_event() because Paolo's earlier net patches have moved it into qemu_flush_queued_packets(). Additional info: I can reproduce the network hang with a tap device doing a iPXE HTTP boot as follows: $ qemu -enable-kvm -m 1024 \ -netdev tap,id=netdev0,script=no,downscript=no \ -device i82559er,netdev=netdev0,romfile=80861209.rom \ -drive if=virtio,cache=none,file=test.img iPXE> ifopen net0 iPXE> config # set static network configuration iPXE> kernel http://mirror.bytemark.co.uk/fedora/linux/releases/17/Fedora/x86_64/os/images/pxeboot/vmlinuz I needed a vanilla iPXE ROM to get to the iPXE prompt. I think the boot prompt has been disabled in the ROMs that ship with QEMU to reduce boot time. During the vmlinuz HTTP download there is a network hang. hw/eepro100.c has reached the end of the rx descriptor list. When the iPXE driver replenishes the rx descriptor list we don't kick the QEMU net subsystem and event loop, thereby leaving the tap netdev without its file descriptor in select(2). Stefan Hajnoczi ] Signed-off-by: Bo Yang Signed-off-by: Stefan Hajnoczi (cherry picked from commit 1069985fb132cd4324fc02d371f1e61492a1823f) Signed-off-by: Michael Tokarev --- hw/eepro100.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/eepro100.c b/hw/eepro100.c index 6279ae3..5b62196 100644 --- a/hw/eepro100.c +++ b/hw/eepro100.c @@ -1036,6 +1036,7 @@ static void eepro100_ru_command(EEPRO100State * s, uint8_t val) } set_ru_state(s, ru_ready); s->ru_offset = e100_read_reg4(s, SCBPointer); +qemu_flush_queued_packets(&s->nic->nc); TRACE(OTHER, logout("val=0x%02x (rx start)\n", val)); break; case RX_RESUME: @@ -1763,7 +1764,8 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size if (rfd_command & COMMAND_EL) { /* EL bit is set, so this was the last frame. */ logout("receive: Running out of frames\n"); -set_ru_state(s, ru_suspended); +set_ru_state(s, ru_no_resources); +eepro100_rnr_interrupt(s); } if (rfd_command & COMMAND_S) { /* S bit is set. */ -- 1.7.10.4
Re: [Qemu-devel] [PATCH] get_tmp_filename: add explicit error message
On 02/04/2013 11:34 AM, Markus Armbruster wrote: > Stefan Hajnoczi writes: > >> On Fri, Feb 01, 2013 at 06:13:51PM +0100, Fabien Chouteau wrote: >>> >>> Signed-off-by: Fabien Chouteau >>> --- >>> block.c | 13 ++--- >>> 1 file changed, 10 insertions(+), 3 deletions(-) >> >> Hi Fabien, >> Please always CC qemu-devel@nongnu.org. All patches must be on >> qemu-devel so that the community can review them - not everyone >> subscribes to qemu-trivial. >> >> Thanks, >> Stefan >> >>> diff --git a/block.c b/block.c >>> index ba67c0d..3bf8eda 100644 >>> --- a/block.c >>> +++ b/block.c >>> @@ -428,9 +428,16 @@ int get_tmp_filename(char *filename, int size) >>> /* GetTempFileName requires that its output buffer (4th param) >>> have length MAX_PATH or greater. */ >>> assert(size >= MAX_PATH); >>> -return (GetTempPath(MAX_PATH, temp_dir) >>> -&& GetTempFileName(temp_dir, "qem", 0, filename) >>> -? 0 : -GetLastError()); >>> +if (GetTempPath(MAX_PATH, temp_dir) == 0) { >>> +fprintf(stderr, "GetTempPath() error: %d\n", GetLastError()); >>> +return -GetLastError(); >>> +} >>> +if (GetTempFileName(temp_dir, "qem", 0, filename) == 0) { >>> +fprintf(stderr, "GetTempFileName(%s) error: %d\n", temp_dir, >>> +GetLastError()); >>> +return -GetLastError(); >>> +} >>> +return 0; >>> #else >>> int fd; >>> const char *tmpdir; > > get_tmp_filename() is not supposed to print to stderr, that's the > caller's job. > Why? The caller doesn't know the difference between Windows/Linux implementation. And the error handling would have to be duplicated. It's not the first time I add error output in Windows code. Specially in block/, when there's an error, the only thing you get is: "operation not permitted". It's not very helpful and you have to dig in the code to find which function failed. -- Fabien Chouteau
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Feb 4, 2013, at 1:30 AM, Vadim Rozenfeld wrote: > On Sun, 2013-02-03 at 15:09 -0600, Anthony Liguori wrote: >> Michael Tokarev writes: >> >>> 03.02.2013 17:23, Yan Vugenfirer wrote: >>> > If it helps, mq changes the config size from 8 to 16 bytes. If the > driver was making an assumption about an 8-byte config size, that's > likely what the problem is. > That's exactly the problem. >>> >>> So what do we do? It isn't nice to break existing setups. >>> Maybe mq can be disabled by default (together with Antony's >>> patch) until fixed drivers will be more widely available? >> >> I've got a patch that does exactly like this. It's pretty ugly though >> because of the relationship between virtio-pci and virtio-net. I'm >> going to try to see if I can find a cleaner way to do it on Monday. >> >> I'm also contemplating just disabling mq by default. Since a special >> command line is needed to enable it anyway (on the backend), having to >> specify mq=on for the device doesn't seem so bad. >> >> But yeah, we don't want Windows guests to break with -M pc by default so >> we should do something to work around it. >> >> N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net >> feature is going to trigger it (not just multiqueue). So while pc-1.3 >> will work forever with this guest image, it's pretty much guaranteed to >> break at some point in the future. >> >>> It's easy to turn off mq by default and turn it on when >>> needed... >>> >>> The problem now is that it isn't obvious why your existing >>> setup breaks when you upgrade. While when you especially >>> play with mq, you may look at options available around it... >> >> If we ever to get virtio2, a really handy feature to have would be a >> driver identification string. Even if was only informative (and not > > Why not just use revision id for that? That's what would be expected from real HW if size of the BAR is changed. > It really can be very useful, at > least for virtio Windows drivers. > BTW, Yan already fixed this problem in the Windows driver code. So we > can make a new build and make it public. But it probably will not be > WHQL'ed in the nearest future. > >> authoritative, we've had a lot of issues like this and being able to >> make work arounds conditional on the driver identification string would >> be nice. >> >> Regards, >> >> Anthony Liguori >> >>> >>> How difficult it is to fix it in win driver? >>> >>> Thanks, >>> >>> /mjt > >
[Qemu-devel] [PATCH 41/60] mips/malta: fix CBUS UART interrupt pin
From: Aurelien Jarno According to the MIPS Malta Developement Platform User's Manual, the i8259 interrupt controller is supposed to be connected to the hardware IRQ0, and the CBUS UART to the hardware interrupt 2. In QEMU they are both connected to hardware interrupt 0, the CBUS UART interrupt being wrong. This patch fixes that. It should be noted that the irq array in QEMU includes the software interrupts, hence env->irq[2] is the first hardware interrupt. Cc: Ralf Baechle Reviewed-by: Eric Johnson Signed-off-by: Aurelien Jarno (cherry picked from commit 68d001928b151a0c50f367c0bdca645b3d5e9ed3) Signed-off-by: Michael Tokarev --- hw/mips_malta.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/mips_malta.c b/hw/mips_malta.c index 4752bb2..337a2b4 100644 --- a/hw/mips_malta.c +++ b/hw/mips_malta.c @@ -854,7 +854,8 @@ void mips_malta_init (ram_addr_t ram_size, be = 0; #endif /* FPGA */ -malta_fpga_init(system_memory, FPGA_ADDRESS, env->irq[2], serial_hds[2]); +/* The CBUS UART is attached to the MIPS CPU INT2 pin, ie interrupt 4 */ +malta_fpga_init(system_memory, FPGA_ADDRESS, env->irq[4], serial_hds[2]); /* Load firmware in flash / BIOS. */ dinfo = drive_get(IF_PFLASH, 0, fl_idx); -- 1.7.10.4
[Qemu-devel] [PATCH 49/60] tap: reset vnet header size on open
From: "Michael S. Tsirkin" For tap, we currently assume the vnet header size is 10 (the default value) but that might not be the case if tap is persistent and has been used by qemu previously. To fix, set host header size in tap device on open. Signed-off-by: Michael S. Tsirkin Tested-by: Alexander Graf Signed-off-by: Stefan Hajnoczi (cherry picked from commit 58ddcd50f30cb5c020bd4f9f36b01ee160a27cac) Signed-off-by: Michael Tokarev --- net/tap.c |7 +++ 1 file changed, 7 insertions(+) diff --git a/net/tap.c b/net/tap.c index f240028..08be84f 100644 --- a/net/tap.c +++ b/net/tap.c @@ -339,6 +339,13 @@ static TAPState *net_tap_fd_init(VLANState *vlan, s->using_vnet_hdr = 0; s->has_ufo = tap_probe_has_ufo(s->fd); tap_set_offload(&s->nc, 0, 0, 0, 0, 0); +/* + * Make sure host header length is set correctly in tap: + * it might have been modified by another instance of qemu. + */ +if (tap_probe_vnet_hdr_len(s->fd, s->host_vnet_hdr_len)) { +tap_fd_set_vnet_hdr_len(s->fd, s->host_vnet_hdr_len); +} tap_read_poll(s, 1); s->vhost_net = NULL; return s; -- 1.7.10.4
Re: [Qemu-devel] [RFC][PATCH]Add timestamp to error message
Stefan Hajnoczi writes: > On Sat, Feb 02, 2013 at 08:31:45AM +0100, Markus Armbruster wrote: >> Seiji Aguchi writes: >> >> > [Issue] >> > When we offer a customer support service and a problem happens >> > in a customer's system, we try to understand the problem by >> > comparing what the customer reports with message logs of the >> > customer's system. >> > >> > In this case, we often need to know when the problem happens. >> > >> > But, currently, there is no timestamp in qemu's error messages. >> > Therefore, we may not be able to understand the problem based on >> > error messages. >> > >> > [Solution] >> > This patch adds a timestamp to qemu's error message logged by >> > error_report(). >> > >> > A logic calculating a time is copied from libvirt, src/util/virtime.c. >> >> Do we really want to add timestamps to error messages unconditionally? >> I don't doubt it's useful in your scenario, but most of the time it's >> just annoying clutter. > > Agreed, I think it should be an option. Also remember that management > tools can pass a pipe as stderr when starting QEMU, and then they can > add their preferred timestamping/log formatting outside of QEMU. That's actually how I'd solve the problem.
[Qemu-devel] [PATCH 31/60] linux-user: fix emulation of getdents
From: "Dmitry V. Levin" In case when TARGET_ABI_BITS == 32 && HOST_LONG_BITS == 64, the last byte of the target dirent structure (aka d_type byte) was never copied from the host dirent structure, thus breaking everything that relies on valid d_type value, e.g. glob(3). Reviewed-by: Peter Maydell Signed-off-by: Dmitry V. Levin Signed-off-by: Riku Voipio (cherry picked from commit 333858b77c2b4f7636257808a77822c58bdd80fe) Signed-off-by: Michael Tokarev --- linux-user/syscall.c | 11 +-- linux-user/syscall_defs.h |8 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 9bf0b28..6444155 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -6959,15 +6959,14 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, tde = target_dirp; while (len > 0) { reclen = de->d_reclen; - treclen = reclen - (2 * (sizeof(long) - sizeof(abi_long))); +tnamelen = reclen - offsetof(struct linux_dirent, d_name); +assert(tnamelen >= 0); +treclen = tnamelen + offsetof(struct target_dirent, d_name); +assert(count1 + treclen <= count); tde->d_reclen = tswap16(treclen); tde->d_ino = tswapal(de->d_ino); tde->d_off = tswapal(de->d_off); - tnamelen = treclen - (2 * sizeof(abi_long) + 2); - if (tnamelen > 256) -tnamelen = 256; -/* XXX: may not be correct */ -pstrcpy(tde->d_name, tnamelen, de->d_name); +memcpy(tde->d_name, de->d_name, tnamelen); de = (struct linux_dirent *)((char *)de + reclen); len -= reclen; tde = (struct target_dirent *)((char *)tde + treclen); diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h index a79b67d..66814af 100644 --- a/linux-user/syscall_defs.h +++ b/linux-user/syscall_defs.h @@ -255,10 +255,10 @@ struct kernel_statfs { }; struct target_dirent { - abi_longd_ino; - abi_longd_off; - unsigned short d_reclen; - chard_name[256]; /* We must not include limits.h! */ +abi_longd_ino; +abi_longd_off; +unsigned short d_reclen; +chard_name[]; }; struct target_dirent64 { -- 1.7.10.4
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Mon, Feb 04, 2013 at 02:00:46PM +0200, Yan Vugenfirer wrote: > > On Feb 4, 2013, at 1:30 AM, Vadim Rozenfeld wrote: > > > On Sun, 2013-02-03 at 15:09 -0600, Anthony Liguori wrote: > >> Michael Tokarev writes: > >> > >>> 03.02.2013 17:23, Yan Vugenfirer wrote: > >>> > > If it helps, mq changes the config size from 8 to 16 bytes. If the > > driver was making an assumption about an 8-byte config size, that's > > likely what the problem is. > > > That's exactly the problem. > >>> > >>> So what do we do? It isn't nice to break existing setups. > >>> Maybe mq can be disabled by default (together with Antony's > >>> patch) until fixed drivers will be more widely available? > >> > >> I've got a patch that does exactly like this. It's pretty ugly though > >> because of the relationship between virtio-pci and virtio-net. I'm > >> going to try to see if I can find a cleaner way to do it on Monday. > >> > >> I'm also contemplating just disabling mq by default. Since a special > >> command line is needed to enable it anyway (on the backend), having to > >> specify mq=on for the device doesn't seem so bad. > >> > >> But yeah, we don't want Windows guests to break with -M pc by default so > >> we should do something to work around it. > >> > >> N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net > >> feature is going to trigger it (not just multiqueue). So while pc-1.3 > >> will work forever with this guest image, it's pretty much guaranteed to > >> break at some point in the future. > >> > >>> It's easy to turn off mq by default and turn it on when > >>> needed... > >>> > >>> The problem now is that it isn't obvious why your existing > >>> setup breaks when you upgrade. While when you especially > >>> play with mq, you may look at options available around it... > >> > >> If we ever to get virtio2, a really handy feature to have would be a > >> driver identification string. Even if was only informative (and not > > > > Why not just use revision id for that? > > That's what would be expected from real HW if size of the BAR is changed. virtio spec is very explicit that revision never changes: "The device must also have a Revision ID of 0 to match this specification." It also explicitly documents the use of feature bits for adding new fields: "In particular, new fields in the device configuration header are indicated by offering a feature bit, so the guest can check before accessing that part of the configuration space. This allows for forwards and backwards compatibility: if the device is enhanced with a new feature bit, older guests will not write that feature bit back to the Guest Features field and it can go into backwards compatibility mode. Similarly, if a guest is enhanced with a feature that the device doesn't support, it will not see that feature bit in the Device Features field and can go into backwards compatibility mode (or, for poor implementations, set the FAILED Device Status bit). " I really don't see how this can be interpreted except as a promise to add fields and a requirement for guest drivers to be forward compatible. > > It really can be very useful, at > > least for virtio Windows drivers. > > BTW, Yan already fixed this problem in the Windows driver code. So we > > can make a new build and make it public. But it probably will not be > > WHQL'ed in the nearest future. > > > >> authoritative, we've had a lot of issues like this and being able to > >> make work arounds conditional on the driver identification string would > >> be nice. > >> > >> Regards, > >> > >> Anthony Liguori > >> > >>> > >>> How difficult it is to fix it in win driver? > >>> > >>> Thanks, > >>> > >>> /mjt > > > >
Re: [Qemu-devel] [PATCH] get_tmp_filename: add explicit error message
Fabien Chouteau writes: > On 02/04/2013 11:34 AM, Markus Armbruster wrote: >> Stefan Hajnoczi writes: >> >>> On Fri, Feb 01, 2013 at 06:13:51PM +0100, Fabien Chouteau wrote: Signed-off-by: Fabien Chouteau --- block.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) >>> >>> Hi Fabien, >>> Please always CC qemu-devel@nongnu.org. All patches must be on >>> qemu-devel so that the community can review them - not everyone >>> subscribes to qemu-trivial. >>> >>> Thanks, >>> Stefan >>> diff --git a/block.c b/block.c index ba67c0d..3bf8eda 100644 --- a/block.c +++ b/block.c @@ -428,9 +428,16 @@ int get_tmp_filename(char *filename, int size) /* GetTempFileName requires that its output buffer (4th param) have length MAX_PATH or greater. */ assert(size >= MAX_PATH); -return (GetTempPath(MAX_PATH, temp_dir) -&& GetTempFileName(temp_dir, "qem", 0, filename) -? 0 : -GetLastError()); +if (GetTempPath(MAX_PATH, temp_dir) == 0) { +fprintf(stderr, "GetTempPath() error: %d\n", GetLastError()); +return -GetLastError(); +} +if (GetTempFileName(temp_dir, "qem", 0, filename) == 0) { +fprintf(stderr, "GetTempFileName(%s) error: %d\n", temp_dir, +GetLastError()); +return -GetLastError(); +} +return 0; #else int fd; const char *tmpdir; >> >> get_tmp_filename() is not supposed to print to stderr, that's the >> caller's job. >> > > Why? The caller doesn't know the difference between Windows/Linux > implementation. And the error handling would have to be duplicated. The function's (implied) contract is to return an error code without printing anything. If you want to change the contract to include reporting the error, you need to implement it both for both arms of the #ifdef. You also have to demonstrate that all callers are happy with the change of contract. Regardless, printing to stderr is wrong. The function can be called on behalf of a monitor command, and then the error needs to be printed to the correct monitor. error_report() can do that for you, and more. > It's not the first time I add error output in Windows code. Specially in > block/, when there's an error, the only thing you get is: "operation not > permitted". It's not very helpful and you have to dig in the code to > find which function failed. Good error reporting is hard. Knowledge about the error and its context gets lost as you move up the call chain. Knowledge about how to report errors gets lost as you move down.
Re: [Qemu-devel] [PATCH qom-cpu-next v3 2/4] target-i386: Split command line parsing out of cpu_x86_register()
Am 04.02.2013 12:14, schrieb Igor Mammedov: > On Sat, 2 Feb 2013 01:37:06 +0100 > Andreas Färber wrote: > >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> index e74802b..ee2fd6b 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c [..] >> @@ -1578,26 +1566,50 @@ out: >> >> X86CPU *cpu_x86_init(const char *cpu_model) >> { >> -X86CPU *cpu; >> +X86CPU *cpu = NULL; >> CPUX86State *env; >> +gchar **model_pieces; >> +char *name, *features; >> Error *error = NULL; >> >> +model_pieces = g_strsplit(cpu_model, ",", 2); >> +if (!model_pieces[0]) { >> +error_setg(&error, "Invalid/empty CPU model name"); >> +goto error; >> +} >> +name = model_pieces[0]; >> +features = model_pieces[1]; >> + >> cpu = X86_CPU(object_new(TYPE_X86_CPU)); >> env = &cpu->env; >> env->cpu_model_str = cpu_model; >> >> -if (cpu_x86_register(cpu, cpu_model) < 0) { >> -object_delete(OBJECT(cpu)); >> -return NULL; >> +if (cpu_x86_register(cpu, name) < 0) { >> +goto error; >> +} >> + >> +cpu_x86_parse_featurestr(cpu, features, &error); >> +if (error) { >> +goto error; >> } >> >> object_property_set_bool(OBJECT(cpu), true, "realized", &error); >> if (error) { >> +goto error; >> +} >> +g_strfreev(model_pieces); >> +return cpu; >> + >> +error: > Could you use common exit path like it's done in cpu_x86_register()? > That allows to avoid cleanup code duplication on exit. Well, we don't want to object_unref() the CPU in the cleanup path, so we would need to set Error* also in the registration case and move it into that code path, that would be possible. Andreas >> +g_strfreev(model_pieces); >> +if (error) { >> +fprintf(stderr, "%s\n", error_get_pretty(error)); >> error_free(error); >> +} >> +if (cpu != NULL) { >> object_delete(OBJECT(cpu)); >> -return NULL; >> } >> -return cpu; >> +return NULL; >> } >> >> #if !defined(CONFIG_USER_ONLY) -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH 18/60] e1000: flush queue whenever can_receive can go from false to true
From: Paolo Bonzini When the guests replenish the receive ring buffer, the network device should flush its queue of pending packets. This is done with qemu_flush_queued_packets. e1000's can_receive can go from false to true when RCTL or RDT are modified. Reported-by: Luigi Rizzo Cc: Stefan Hajnoczi Cc: Jan Kiszka Signed-off-by: Paolo Bonzini Reviewed-by: Amos Kong Signed-off-by: Stefan Hajnoczi (cherry picked from commit e8b4c680b41bd960ecccd9ff076b7b058e0afcd4) Signed-off-by: Michael Tokarev --- hw/e1000.c |4 1 file changed, 4 insertions(+) diff --git a/hw/e1000.c b/hw/e1000.c index 4573f13..43d933a 100644 --- a/hw/e1000.c +++ b/hw/e1000.c @@ -295,6 +295,7 @@ set_rx_control(E1000State *s, int index, uint32_t val) s->rxbuf_min_shift = ((val / E1000_RCTL_RDMTS_QUAT) & 3) + 1; DBGOUT(RX, "RCTL: %d, mac_reg[RCTL] = 0x%x\n", s->mac_reg[RDT], s->mac_reg[RCTL]); +qemu_flush_queued_packets(&s->nic->nc); } static void @@ -926,6 +927,9 @@ set_rdt(E1000State *s, int index, uint32_t val) { s->check_rxov = 0; s->mac_reg[index] = val & 0x; +if (e1000_has_rxbufs(s, 1)) { +qemu_flush_queued_packets(&s->nic->nc); +} } static void -- 1.7.10.4
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
On Mon, 2013-02-04 at 14:22 +0200, Michael S. Tsirkin wrote: > On Mon, Feb 04, 2013 at 02:00:46PM +0200, Yan Vugenfirer wrote: > > > > On Feb 4, 2013, at 1:30 AM, Vadim Rozenfeld wrote: > > > > > On Sun, 2013-02-03 at 15:09 -0600, Anthony Liguori wrote: > > >> Michael Tokarev writes: > > >> > > >>> 03.02.2013 17:23, Yan Vugenfirer wrote: > > >>> > > > If it helps, mq changes the config size from 8 to 16 bytes. If the > > > driver was making an assumption about an 8-byte config size, that's > > > likely what the problem is. > > > > > That's exactly the problem. > > >>> > > >>> So what do we do? It isn't nice to break existing setups. > > >>> Maybe mq can be disabled by default (together with Antony's > > >>> patch) until fixed drivers will be more widely available? > > >> > > >> I've got a patch that does exactly like this. It's pretty ugly though > > >> because of the relationship between virtio-pci and virtio-net. I'm > > >> going to try to see if I can find a cleaner way to do it on Monday. > > >> > > >> I'm also contemplating just disabling mq by default. Since a special > > >> command line is needed to enable it anyway (on the backend), having to > > >> specify mq=on for the device doesn't seem so bad. > > >> > > >> But yeah, we don't want Windows guests to break with -M pc by default so > > >> we should do something to work around it. > > >> > > >> N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net > > >> feature is going to trigger it (not just multiqueue). So while pc-1.3 > > >> will work forever with this guest image, it's pretty much guaranteed to > > >> break at some point in the future. > > >> > > >>> It's easy to turn off mq by default and turn it on when > > >>> needed... > > >>> > > >>> The problem now is that it isn't obvious why your existing > > >>> setup breaks when you upgrade. While when you especially > > >>> play with mq, you may look at options available around it... > > >> > > >> If we ever to get virtio2, a really handy feature to have would be a > > >> driver identification string. Even if was only informative (and not > > > > > > Why not just use revision id for that? > > > > That's what would be expected from real HW if size of the BAR is changed. > > virtio spec is very explicit that revision never changes: > "The device must also have a Revision ID of 0 to match this > specification." With all my respect to the virtio spec, let me point that Windows lives by different rules: http://msdn.microsoft.com/en-us/library/windows/hardware/gg463287.aspx > It also explicitly documents the use of feature bits for > adding new fields: > > "In particular, new fields in the device configuration header are > indicated by offering a feature bit, so the guest can check before > accessing that part of the configuration space. > > This allows for forwards and backwards compatibility: if the device is > enhanced with a new feature bit, older guests will not write that > feature bit back to the Guest Features field and it can go into > backwards compatibility mode. Similarly, if a guest is enhanced with a > feature that the device doesn't support, it will not see that feature > bit in the Device Features field and can go into backwards compatibility > mode (or, for poor implementations, set the FAILED Device Status bit). > " > > I really don't see how this can be interpreted except as a > promise to add fields and a requirement for guest drivers > to be forward compatible. > > > > It really can be very useful, at > > > least for virtio Windows drivers. > > > BTW, Yan already fixed this problem in the Windows driver code. So we > > > can make a new build and make it public. But it probably will not be > > > WHQL'ed in the nearest future. > > > > > >> authoritative, we've had a lot of issues like this and being able to > > >> make work arounds conditional on the driver identification string would > > >> be nice. > > >> > > >> Regards, > > >> > > >> Anthony Liguori > > >> > > >>> > > >>> How difficult it is to fix it in win driver? > > >>> > > >>> Thanks, > > >>> > > >>> /mjt > > > > > >
Re: [Qemu-devel] [PATCH v3 01/10] main-loop: fix select_ret uninitialized variable warning
On 02/04/13 13:12, Stefan Hajnoczi wrote: > Signed-off-by: Stefan Hajnoczi > --- > main-loop.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/main-loop.c b/main-loop.c > index 6f52ac3..d0d8fe4 100644 > --- a/main-loop.c > +++ b/main-loop.c > @@ -330,7 +330,8 @@ void qemu_fd_register(int fd) > static int os_host_main_loop_wait(uint32_t timeout) > { > GMainContext *context = g_main_context_default(); > -int select_ret, g_poll_ret, ret, i; > +int select_ret = 0; > +int g_poll_ret, ret, i; > PollingEntry *pe; > WaitObjects *w = &wait_objects; > gint poll_timeout; Reviewed-by: Laszlo Ersek
Re: [Qemu-devel] [PATCH 1/4] migration: change initial value of expected_downtime
On 02/01/2013 02:32 PM, Juan Quintela wrote: > 0 is a very bad initial value, what we are trying to get is > max_downtime, so that is a much better estimation. > > Signed-off-by: Juan Quintela > --- > migration.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/migration.c b/migration.c > index 77c1971..d86946e 100644 > --- a/migration.c > +++ b/migration.c > @@ -782,6 +782,8 @@ void migrate_fd_connect(MigrationState *s) > s->buffer = NULL; > s->buffer_size = 0; > s->buffer_capacity = 0; > +/* This is a best 1st approximation. ns to ms */ > +s->expected_downtime = max_downtime/100; > > s->xfer_limit = s->bandwidth_limit / XFER_LIMIT_RATIO; > s->complete = false; > Reviewed-by: Orit Wasserman
Re: [Qemu-devel] [PATCH 2/4] migration: calculate end time after we have sent the data
On 02/01/2013 02:32 PM, Juan Quintela wrote: > Signed-off-by: Juan Quintela > --- > migration.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/migration.c b/migration.c > index d86946e..67abd12 100644 > --- a/migration.c > +++ b/migration.c > @@ -681,7 +681,7 @@ static void *buffered_file_thread(void *opaque) > qemu_mutex_unlock_iothread(); > > while (true) { > -int64_t current_time = qemu_get_clock_ms(rt_clock); > +int64_t current_time; > uint64_t pending_size; > > qemu_mutex_lock_iothread(); > @@ -735,6 +735,7 @@ static void *buffered_file_thread(void *opaque) > } > } > qemu_mutex_unlock_iothread(); > +current_time = qemu_get_clock_ms(rt_clock); > if (current_time >= initial_time + BUFFER_DELAY) { > uint64_t transferred_bytes = s->bytes_xfer; > uint64_t time_spent = current_time - initial_time; > Reviewed-by: Orit Wasserman
[Qemu-devel] [PATCH 09/60] ahci: properly reset PxCMD on HBA reset
From: Jason Baron While testing q35, I found that windows 7 (specifically, windows 7 ultimate with sp1 x64), wouldn't install because it can't find the cdrom or disk drive. The failure message is: 'A required cd/dvd device driver is missing. If you have a driver floppy disk, CD, DVD, or USB flash drive, please insert it now.' This can also be reproduced on piix by adding an ahci controller, and observing that windows 7 does not see any devices behind it. The problem is that when windows issues a HBA reset, qemu does not reset the individual ports' PxCMD register. Windows 7 then reads back the PxCMD register and presumably assumes that the ahci controller has already been initialized. Windows then never sets up the PxIE register to enable interrupts, and thus it never gets irqs back when it sends ata device inquiry commands. This change brings qemu into ahci 1.3 specification compliance. Section 10.4.3 HBA Reset: " When GHC.HR is set to '1', GHC.AE, GHC.IE, the IS register, and all port register fields (except PxFB/PxFBU/PxCLB/PxCLBU) that are not HwInit in the HBA's register memory space are reset. " I've also re-tested Fedora 16 and 17 to verify that they continue to work with this change. Signed-off-by: Jason Baron Acked-by: Alexander Graf Signed-off-by: Kevin Wolf (cherry picked from commit 2a4f4f34e6fe55f4c82507c3e7ec9b58c2e24ad4) Conflicts: hw/ide/ahci.c --- hw/ide/ahci.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 267198e..1b5b5bf 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1168,7 +1168,6 @@ void ahci_init(AHCIState *s, DeviceState *qdev, int ports) ad->port_no = i; ad->port.dma = &ad->dma; ad->port.dma->ops = &ahci_dma_ops; -ad->port_regs.cmd = PORT_CMD_SPIN_UP | PORT_CMD_POWER_ON; } } @@ -1193,6 +1192,7 @@ void ahci_reset(void *opaque) pr->irq_stat = 0; pr->irq_mask = 0; pr->scr_ctl = 0; +pr->cmd = PORT_CMD_SPIN_UP | PORT_CMD_POWER_ON; ahci_reset_port(&d->ahci, i); } } -- 1.7.10.4
Re: [Qemu-devel] [PATCH 3/4] migration: don't account sleep time for calculating bandwidth
On 02/01/2013 02:32 PM, Juan Quintela wrote: > While we are sleeping we are not sending, so we should not use that > time to estimate our bandwidth. Juan, Maybe I missing something here but the sleep time is cause by the fact we limit the migration bandwidth. So the available bandwidth should take it into consideration? we still cape the bandwidth in the last stage. Regards, Orit > > Signed-off-by: Juan Quintela > --- > migration.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/migration.c b/migration.c > index 67abd12..64e75ca 100644 > --- a/migration.c > +++ b/migration.c > @@ -666,6 +666,7 @@ static void *buffered_file_thread(void *opaque) > { > MigrationState *s = opaque; > int64_t initial_time = qemu_get_clock_ms(rt_clock); > +int64_t sleep_time = 0; > int64_t max_size = 0; > bool last_round = false; > int ret; > @@ -738,7 +739,7 @@ static void *buffered_file_thread(void *opaque) > current_time = qemu_get_clock_ms(rt_clock); > if (current_time >= initial_time + BUFFER_DELAY) { > uint64_t transferred_bytes = s->bytes_xfer; > -uint64_t time_spent = current_time - initial_time; > +uint64_t time_spent = current_time - initial_time - sleep_time; > double bandwidth = transferred_bytes / time_spent; > max_size = bandwidth * migrate_max_downtime() / 100; > > @@ -747,11 +748,13 @@ static void *buffered_file_thread(void *opaque) > transferred_bytes, time_spent, bandwidth, max_size); > > s->bytes_xfer = 0; > +sleep_time = 0; > initial_time = current_time; > } > if (!last_round && (s->bytes_xfer >= s->xfer_limit)) { > /* usleep expects microseconds */ > g_usleep((initial_time + BUFFER_DELAY - current_time)*1000); > +sleep_time += qemu_get_clock_ms(rt_clock) - current_time; > } > ret = buffered_flush(s); > if (ret < 0) { >
[Qemu-devel] [PATCH 0/4] char: flow control: fixes, virtio-serial flow control
Hi Anthony, This series is based on top of your char-flow.2 branch on github. Patches 1 and 2 fix return values, and make the series work properly. These small patches should ideally be folded in the patches that introduced the code. Patch 3 adds gio watch support for unix and tcp sockets. Patch 4 adds flow control to virtio-serial ports. With these applied, I tested flow control with virtio-serial ports with a unix socket backend, and things are working fine. Amit Shah (4): char: poll a frontend only if it's writable char: fix return value of can_read functions on no input char: add gio watch fn for tcp backends virtio: console: add flow control hw/virtio-console.c | 13 + qemu-char.c | 15 +++ 2 files changed, 24 insertions(+), 4 deletions(-) -- 1.8.1
[Qemu-devel] [PATCH 05/60] fix CONFIG_QEMU_HELPERDIR generation again
commit 38f419f35225 fixed a breakage with CONFIG_QEMU_HELPERDIR which has been introduced by 8bf188aa18ef7a8. But while techinically that fix has been correct, all other similar variables are handled differently. Make it consistent, and let scripts/create_config expand and capitalize the variable properly like for all other qemu_*dir variables. Signed-off-by: Michael Tokarev (cherry picked from commit f354b1a1ee7a1c72d51b42808724a2b10eec315f) --- configure |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index d5c0a47..4b4991c 100755 --- a/configure +++ b/configure @@ -3072,7 +3072,7 @@ echo "sysconfdir=$sysconfdir" >> $config_host_mak echo "qemu_confdir=$qemu_confdir" >> $config_host_mak echo "qemu_datadir=$qemu_datadir" >> $config_host_mak echo "qemu_docdir=$qemu_docdir" >> $config_host_mak -echo "CONFIG_QEMU_HELPERDIR=\"`eval echo $libexecdir`\"" >> $config_host_mak +echo "qemu_helperdir=$libexecdir" >> $config_host_mak echo "ARCH=$ARCH" >> $config_host_mak if test "$debug_tcg" = "yes" ; then -- 1.7.10.4
[Qemu-devel] [PATCH 1/4] char: poll a frontend only if it's writable
If the frontend reports it's not writable, don't start polling it for data yet. Signed-off-by: Amit Shah --- qemu-char.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-char.c b/qemu-char.c index 2b714cf..5731d02 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -569,7 +569,7 @@ static gboolean io_watch_poll_prepare(GSource *source, gint *timeout_) iwp->max_size = iwp->fd_can_read(iwp->opaque); if (iwp->max_size == 0) { -return TRUE; +return FALSE; } return g_io_watch_funcs.prepare(source, timeout_); -- 1.8.1
[Qemu-devel] [PATCH 2/4] char: fix return value of can_read functions on no input
When the can_read functions report the frontend isn't ready to accept data yet, return 'FALSE' instead of 'TRUE'. Signed-off-by: Amit Shah --- qemu-char.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index 5731d02..7fa9372 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -1067,7 +1067,7 @@ static gboolean pty_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) if (len > s->read_bytes) len = s->read_bytes; if (len == 0) -return TRUE; +return FALSE; status = g_io_channel_read_chars(s->fd, (gchar *)buf, len, &size, NULL); if (status != G_IO_STATUS_NORMAL) { pty_chr_state(chr, 0); @@ -2237,7 +2237,7 @@ static gboolean udp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) GIOStatus status; if (s->max_size == 0) -return TRUE; +return FALSE; status = g_io_channel_read_chars(s->chan, (gchar *)s->buf, sizeof(s->buf), &bytes_read, NULL); s->bufcnt = bytes_read; @@ -2492,7 +2492,7 @@ static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) int len, size; if (!s->connected || s->max_size <= 0) { -return TRUE; +return FALSE; } len = sizeof(buf); if (len > s->max_size) -- 1.8.1
Re: [Qemu-devel] [PATCH 4/4] migration: calculate expected_downtime
On 02/01/2013 02:32 PM, Juan Quintela wrote: > We removed the calculation in commit e4ed1541ac9413eac494a03532e34beaf8a7d1c5 > > Now we add it back. We need to create dirty_bytes_rate because we > can't include cpu-all.h from migration.c, and there is no other way to > include TARGET_PAGE_SIZE. > > Signed-off-by: Juan Quintela > --- > arch_init.c | 1 + > include/migration/migration.h | 1 + > migration.c | 5 + > 3 files changed, 7 insertions(+) > > diff --git a/arch_init.c b/arch_init.c > index dada6de..634490a 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -414,6 +414,7 @@ static void migration_bitmap_sync(void) > if (end_time > start_time + 1000) { > s->dirty_pages_rate = num_dirty_pages_period * 1000 > / (end_time - start_time); > +s->dirty_bytes_rate = s->dirty_pages_rate * TARGET_PAGE_SIZE; > start_time = end_time; > num_dirty_pages_period = 0; > } > diff --git a/include/migration/migration.h b/include/migration/migration.h > index a8c9639..d121409 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -51,6 +51,7 @@ struct MigrationState > int64_t downtime; > int64_t expected_downtime; > int64_t dirty_pages_rate; > +int64_t dirty_bytes_rate; > bool enabled_capabilities[MIGRATION_CAPABILITY_MAX]; > int64_t xbzrle_cache_size; > bool complete; > diff --git a/migration.c b/migration.c > index 64e75ca..4eca42e 100644 > --- a/migration.c > +++ b/migration.c > @@ -746,6 +746,11 @@ static void *buffered_file_thread(void *opaque) > DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64 > " bandwidth %g max_size %" PRId64 "\n", > transferred_bytes, time_spent, bandwidth, max_size); > +/* if we haven't sent anything, we don't want to recalculate > + 1 is a small enough number for our purposes */ > +if (s->dirty_bytes_rate && transferred_bytes > 1) { > +s->expected_downtime = s->dirty_bytes_rate / bandwidth; > +} > > s->bytes_xfer = 0; > sleep_time = 0; > Reviewed-by: Orit Wasserman
Re: [Qemu-devel] [PATCH qom-cpu-next v3 3/4] target-i386: Slim conversion to X86CPU subclasses
Am 04.02.2013 12:08, schrieb Igor Mammedov: > On Sat, 2 Feb 2013 01:37:07 +0100 > Andreas Färber wrote: > >> Move x86_def_t definition to header and embed into X86CPUClass. >> Register types per built-in model definition. >> >> Move version initialization from x86_cpudef_setup() to class_init. >> >> Inline cpu_x86_register() into the X86CPU initfn. >> Since instance_init cannot reports errors, drop error handling. >> >> Replace cpu_x86_find_by_name() with x86_cpu_class_by_name(). >> Move KVM host vendor override from cpu_x86_find_by_name() to the initfn. >> >> Register host-{i386,x86_64}-cpu type from KVM code to avoid #ifdefs. >> Make kvm_cpu_fill_host() a class_init and inline cpu_x86_fill_model_id(). >> >> Let kvm_check_features_against_host() obtain host-{i386,86_64}-cpu for >> comparison. >> >> Signed-off-by: Andreas Färber >> --- >> target-i386/cpu-qom.h | 24 >> target-i386/cpu.c | 324 >> + target-i386/cpu.h >> |2 - target-i386/kvm.c | 93 ++ >> 4 Dateien geändert, 228 Zeilen hinzugefügt(+), 215 Zeilen entfernt(-) >> >> diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h >> index 48e6b54..80bf72d 100644 >> --- a/target-i386/cpu-qom.h >> +++ b/target-i386/cpu-qom.h >> @@ -30,6 +30,27 @@ >> #define TYPE_X86_CPU "i386-cpu" >> #endif >> >> +#define TYPE_HOST_X86_CPU "host-" TYPE_X86_CPU >> + >> +typedef struct x86_def_t { >> +const char *name; >> +uint32_t level; >> +/* vendor is zero-terminated, 12 character ASCII string */ >> +char vendor[CPUID_VENDOR_SZ + 1]; >> +int family; >> +int model; >> +int stepping; >> +uint32_t features, ext_features, ext2_features, ext3_features; >> +uint32_t kvm_features, svm_features; >> +uint32_t xlevel; >> +char model_id[48]; >> +/* Store the results of Centaur's CPUID instructions */ >> +uint32_t ext4_features; >> +uint32_t xlevel2; >> +/* The feature bits on CPUID[EAX=7,ECX=0].EBX */ >> +uint32_t cpuid_7_0_ebx_features; >> +} x86_def_t; >> + >> #define X86_CPU_CLASS(klass) \ >> OBJECT_CLASS_CHECK(X86CPUClass, (klass), TYPE_X86_CPU) >> #define X86_CPU(obj) \ >> @@ -41,6 +62,7 @@ >> * X86CPUClass: >> * @parent_realize: The parent class' realize handler. >> * @parent_reset: The parent class' reset handler. >> + * @info: Model-specific data. >> * >> * An x86 CPU model or family. >> */ >> @@ -51,6 +73,8 @@ typedef struct X86CPUClass { >> >> DeviceRealize parent_realize; >> void (*parent_reset)(CPUState *cpu); >> + >> +x86_def_t info; > Is it necessary to embed it here, could pointer to corresponding static array > be used here? > That way one could avoid extra memcpy in class_init(). That would require an allocation for -cpu host, which is undesired. x86_def_t is supposed to die, forgot to add a TODO comment like on ppc. Patches to do that are being being polished on qom-cpu-ppc. >> } X86CPUClass; >> >> /** >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> index ee2fd6b..6c95740 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c >> @@ -346,25 +346,6 @@ static void add_flagname_to_bitmaps(const char >> *flagname, } >> } >> >> -typedef struct x86_def_t { >> -const char *name; >> -uint32_t level; >> -/* vendor is zero-terminated, 12 character ASCII string */ >> -char vendor[CPUID_VENDOR_SZ + 1]; >> -int family; >> -int model; >> -int stepping; >> -uint32_t features, ext_features, ext2_features, ext3_features; >> -uint32_t kvm_features, svm_features; >> -uint32_t xlevel; >> -char model_id[48]; >> -/* Store the results of Centaur's CPUID instructions */ >> -uint32_t ext4_features; >> -uint32_t xlevel2; >> -/* The feature bits on CPUID[EAX=7,ECX=0].EBX */ >> -uint32_t cpuid_7_0_ebx_features; >> -} x86_def_t; >> - >> #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) >> #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \ >>CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC) >> @@ -868,86 +849,6 @@ static x86_def_t builtin_x86_defs[] = { >> }, >> }; >> >> -#ifdef CONFIG_KVM >> -static int cpu_x86_fill_model_id(char *str) >> -{ >> -uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; >> -int i; >> - >> -for (i = 0; i < 3; i++) { >> -host_cpuid(0x8002 + i, 0, &eax, &ebx, &ecx, &edx); >> -memcpy(str + i * 16 + 0, &eax, 4); >> -memcpy(str + i * 16 + 4, &ebx, 4); >> -memcpy(str + i * 16 + 8, &ecx, 4); >> -memcpy(str + i * 16 + 12, &edx, 4); >> -} >> -return 0; >> -} >> -#endif >> - >> -/* Fill a x86_def_t struct with information about the host CPU, and >> - * the CPU features supported by the host hardware + host kernel >> - * >> - * This function may be called only if KVM is enabled. >> - */ >> -static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) >> -{ >> -#ifdef CONFIG_KVM >> -KVMState
[Qemu-devel] [PATCH 30/60] MIPS: Correct FCR0 initialization
From: Nathan Froyd This change addresses a problem where QEMU incorrectly traps on floating-point MADD group instructions with SIGILL, at least while emulating MIPS32r2 processors. These instructions use the COP1X major opcode and include ones like: madd.d $f2,$f4,$f2,$f6 Here's Nathan's original analysis of the problem: "QEMU essentially does: d = find_cpu (cpu_string) // get CPU definition fpu_init (env, d) // initialize fpu state (init FCR0, basically) cpu_reset (env) ...and the cpu_reset call clears all interesting state that fpu_init setup, then proceeds to reinitialize all the CP0 registers...but not FCR0." I have verified this change with system emulation running the GDB test suite for the mips-sde-elf target (o32, big endian, 24Kf CPU emulated), there were 55 progressions and no regressions. Signed-off-by: Maciej W. Rozycki Reviewed-by: Richard Henderson Signed-off-by: Blue Swirl (cherry picked from commit f1cb0951c5298753652a73cfd8efc0b1a82f37de) Signed-off-by: Michael Tokarev --- target-mips/translate.c |1 + 1 file changed, 1 insertion(+) diff --git a/target-mips/translate.c b/target-mips/translate.c index 5ed58f6..8ff1fab 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -12783,6 +12783,7 @@ void cpu_state_reset(CPUMIPSState *env) env->CP0_SRSConf3 = env->cpu_model->CP0_SRSConf3; env->CP0_SRSConf4_rw_bitmask = env->cpu_model->CP0_SRSConf4_rw_bitmask; env->CP0_SRSConf4 = env->cpu_model->CP0_SRSConf4; +env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0; env->insn_flags = env->cpu_model->insn_flags; #if defined(CONFIG_USER_ONLY) -- 1.7.10.4
[Qemu-devel] [PATCH 19/60] fix entry pointer for ELF kernels loaded with -kernel option
From: Henning Schild Find a hopefully proper patch attached. Take it or leave it. Reviewed-by: Kevin Wolf Signed-off-by: Henning Schild Signed-off-by: Aurelien Jarno (cherry picked from commit 7e9c7ffe9fd9dfc3d0168dd584936db8144b230b) Signed-off-by: Michael Tokarev --- hw/elf_ops.h | 11 +++ 1 file changed, 11 insertions(+) diff --git a/hw/elf_ops.h b/hw/elf_ops.h index fa65ce2..731a983 100644 --- a/hw/elf_ops.h +++ b/hw/elf_ops.h @@ -269,6 +269,17 @@ static int glue(load_elf, SZ)(const char *name, int fd, addr = ph->p_paddr; } +/* the entry pointer in the ELF header is a virtual + * address, if the text segments paddr and vaddr differ + * we need to adjust the entry */ +if (pentry && !translate_fn && +ph->p_vaddr != ph->p_paddr && +ehdr.e_entry >= ph->p_vaddr && +ehdr.e_entry < ph->p_vaddr + ph->p_filesz && +ph->p_flags & PF_X) { +*pentry = ehdr.e_entry - ph->p_vaddr + ph->p_paddr; +} + snprintf(label, sizeof(label), "phdr #%d: %s", i, name); rom_add_blob_fixed(label, data, mem_size, addr); -- 1.7.10.4
Re: [Qemu-devel] [buildbot patch 4/6] use --disable-debug-info
On Mon, Feb 04, 2013 at 10:59:33AM +0100, Gerd Hoffmann wrote: > On 02/04/13 10:26, Stefan Hajnoczi wrote: > > On Fri, Feb 01, 2013 at 03:02:23PM +0100, Gerd Hoffmann wrote: > >> Signed-off-by: Gerd Hoffmann > >> --- > >> qemu-master.cfg |6 -- > >> 1 file changed, 4 insertions(+), 2 deletions(-) > > > > What is the rationale for these changes: > > > > --disable-debug-info to save buildslave disk space? > > Yes. > > > Remove CFLAGS=-O2? > > That was used to remove '-g' before we had --disable-debug-info, so it > isn't needed any more. > > So there doesn't change much, except that the "make check" tests now are > build without debug info too. Thanks for explaining. Stefan
[Qemu-devel] [PATCH v3 02/10] main-loop: switch to g_poll() on POSIX hosts
Use g_poll(3) instead of select(2). Well, this is kind of a cheat. It's true that we're now using g_poll(3) on POSIX hosts but the *_fill() and *_poll() functions are still using rfds/wfds/xfds. We've set the scene to start converting *_fill() and *_poll() functions step-by-step until no more rfds/wfds/xfds users remain. Then we'll drop the temporary gpollfds_from_select() and gpollfds_to_select() functions and be left with native g_poll(2). On Windows things are a little crazy: convert from rfds/wfds/xfds to GPollFDs, back to rfds/wfds/xfds, call select(2), rfds/wfds/xfds back to GPollFDs, and finally back to rfds/wfds/xfds again. This is only temporary and keeps the Windows build working through the following patches. We'll drop this excessive conversion later and be left with a single GPollFDs -> select(2) -> GPollFDs sequence that allows Windows to use select(2) while the rest of QEMU only knows about GPollFD. Signed-off-by: Stefan Hajnoczi --- main-loop.c | 135 1 file changed, 127 insertions(+), 8 deletions(-) diff --git a/main-loop.c b/main-loop.c index d0d8fe4..f1dcd14 100644 --- a/main-loop.c +++ b/main-loop.c @@ -117,6 +117,8 @@ void qemu_notify_event(void) aio_notify(qemu_aio_context); } +static GArray *gpollfds; + int qemu_init_main_loop(void) { int ret; @@ -133,6 +135,7 @@ int qemu_init_main_loop(void) return ret; } +gpollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD)); qemu_aio_context = aio_context_new(); src = aio_get_g_source(qemu_aio_context); g_source_attach(src, NULL); @@ -146,6 +149,62 @@ static GPollFD poll_fds[1024 * 2]; /* this is probably overkill */ static int n_poll_fds; static int max_priority; +/* Load rfds/wfds/xfds into gpollfds. Will be removed a few commits later. */ +static void gpollfds_from_select(void) +{ +int fd; +for (fd = 0; fd <= nfds; fd++) { +int events = 0; +if (FD_ISSET(fd, &rfds)) { +events |= G_IO_IN | G_IO_HUP | G_IO_ERR; +} +if (FD_ISSET(fd, &wfds)) { +events |= G_IO_OUT | G_IO_ERR; +} +if (FD_ISSET(fd, &xfds)) { +events |= G_IO_PRI; +} +if (events) { +GPollFD pfd = { +.fd = fd, +.events = events, +}; +g_array_append_val(gpollfds, pfd); +} +} +} + +/* Store gpollfds revents into rfds/wfds/xfds. Will be removed a few commits + * later. + */ +static void gpollfds_to_select(int ret) +{ +int i; + +FD_ZERO(&rfds); +FD_ZERO(&wfds); +FD_ZERO(&xfds); + +if (ret <= 0) { +return; +} + +for (i = 0; i < gpollfds->len; i++) { +int fd = g_array_index(gpollfds, GPollFD, i).fd; +int revents = g_array_index(gpollfds, GPollFD, i).revents; + +if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR)) { +FD_SET(fd, &rfds); +} +if (revents & (G_IO_OUT | G_IO_ERR)) { +FD_SET(fd, &wfds); +} +if (revents & G_IO_PRI) { +FD_SET(fd, &xfds); +} +} +} + #ifndef _WIN32 static void glib_select_fill(int *max_fd, fd_set *rfds, fd_set *wfds, fd_set *xfds, uint32_t *cur_timeout) @@ -212,22 +271,22 @@ static void glib_select_poll(fd_set *rfds, fd_set *wfds, fd_set *xfds, static int os_host_main_loop_wait(uint32_t timeout) { -struct timeval tv, *tvarg = NULL; int ret; glib_select_fill(&nfds, &rfds, &wfds, &xfds, &timeout); -if (timeout < UINT32_MAX) { -tvarg = &tv; -tv.tv_sec = timeout / 1000; -tv.tv_usec = (timeout % 1000) * 1000; -} - if (timeout > 0) { qemu_mutex_unlock_iothread(); } -ret = select(nfds + 1, &rfds, &wfds, &xfds, tvarg); +/* We'll eventually drop fd_set completely. But for now we still have + * *_fill() and *_poll() functions that use rfds/wfds/xfds. + */ +gpollfds_from_select(); + +ret = g_poll((GPollFD *)gpollfds->data, gpollfds->len, timeout); + +gpollfds_to_select(ret); if (timeout > 0) { qemu_mutex_lock_iothread(); @@ -327,6 +386,55 @@ void qemu_fd_register(int fd) FD_CONNECT | FD_WRITE | FD_OOB); } +static int pollfds_fill(GArray *pollfds, fd_set *rfds, fd_set *wfds, +fd_set *xfds) +{ +int nfds = -1; +int i; + +for (i = 0; i < pollfds->len; i++) { +GPollFD *pfd = &g_array_index(pollfds, GPollFD, i); +int fd = pfd->fd; +int events = pfd->events; +if (events & (G_IO_IN | G_IO_HUP | G_IO_ERR)) { +FD_SET(fd, rfds); +nfds = MAX(nfds, fd); +} +if (events & (G_IO_OUT | G_IO_ERR)) { +FD_SET(fd, wfds); +nfds = MAX(nfds, fd); +} +if (events & G_IO_PRI) { +FD_SET(fd, xfds); +nfds = MAX(nfds, fd); +
[Qemu-devel] [PATCH 11/60] net: add -netdev options to man page
From: Stefan Hajnoczi Document the -netdev syntax which supercedes the older -net syntax. This patch is a first step to making -netdev prominent in the QEMU manual. Reported-by: Anatoly Techtonik Signed-off-by: Stefan Hajnoczi (cherry picked from commit 08d12022c7f1aba6a75150659c6e4c9dff23) Signed-off-by: Michael Tokarev --- qemu-options.hx |7 +++ 1 file changed, 7 insertions(+) diff --git a/qemu-options.hx b/qemu-options.hx index 8b66264..25633bb 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1318,6 +1318,7 @@ Valid values for @var{type} are Not all devices are supported on all targets. Use -net nic,model=? for a list of available devices for your target. +@item -netdev user,id=@var{id}[,@var{option}][,@var{option}][,...] @item -net user[,@var{option}][,@var{option}][,...] Use the user mode network stack which requires no administrator privilege to run. Valid options are: @@ -1326,6 +1327,7 @@ privilege to run. Valid options are: @item vlan=@var{n} Connect user mode stack to VLAN @var{n} (@var{n} = 0 is the default). +@item id=@var{id} @item name=@var{name} Assign symbolic name for use in monitor commands. @@ -1431,6 +1433,7 @@ processed and applied to -net user. Mixing them with the new configuration syntax gives undefined results. Their use for new applications is discouraged as they will be removed from future versions. +@item -netdev tap,id=@var{id}[,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,helper=@var{helper}] @item -net tap[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,helper=@var{helper}] Connect the host TAP network interface @var{name} to VLAN @var{n}. @@ -1470,6 +1473,7 @@ qemu-system-i386 linux.img \ -net nic -net tap,"helper=/usr/local/libexec/qemu-bridge-helper" @end example +@item -netdev bridge,id=@var{id}[,br=@var{bridge}][,helper=@var{helper}] @item -net bridge[,vlan=@var{n}][,name=@var{name}][,br=@var{bridge}][,helper=@var{helper}] Connect a host TAP network interface to a host bridge device. @@ -1492,6 +1496,7 @@ qemu-system-i386 linux.img -net bridge -net nic,model=virtio qemu-system-i386 linux.img -net bridge,br=qemubr0 -net nic,model=virtio @end example +@item -netdev socket,id=@var{id}[,fd=@var{h}][,listen=[@var{host}]:@var{port}][,connect=@var{host}:@var{port}] @item -net socket[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}] [,listen=[@var{host}]:@var{port}][,connect=@var{host}:@var{port}] Connect the VLAN @var{n} to a remote VLAN in another QEMU virtual @@ -1514,6 +1519,7 @@ qemu-system-i386 linux.img \ -net socket,connect=127.0.0.1:1234 @end example +@item -netdev socket,id=@var{id}[,fd=@var{h}][,mcast=@var{maddr}:@var{port}[,localaddr=@var{addr}]] @item -net socket[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,mcast=@var{maddr}:@var{port}[,localaddr=@var{addr}]] Create a VLAN @var{n} shared with another QEMU virtual @@ -1565,6 +1571,7 @@ qemu-system-i386 linux.img \ -net socket,mcast=239.192.168.1:1102,localaddr=1.2.3.4 @end example +@item -netdev vde,id=@var{id}[,sock=@var{socketpath}][,port=@var{n}][,group=@var{groupname}][,mode=@var{octalmode}] @item -net vde[,vlan=@var{n}][,name=@var{name}][,sock=@var{socketpath}] [,port=@var{n}][,group=@var{groupname}][,mode=@var{octalmode}] Connect VLAN @var{n} to PORT @var{n} of a vde switch running on host and listening for incoming connections on @var{socketpath}. Use GROUP @var{groupname} -- 1.7.10.4
[Qemu-devel] [PATCH 40/60] nbd: fixes to read-only handling
From: Paolo Bonzini We do not need BLKROSET if the kernel supports setting flags. Also, always do BLKROSET even for a read-write export, otherwise the read-only state remains "sticky" after the invocation of "qemu-nbd -r". Signed-off-by: Paolo Bonzini (cherry picked from commit c8969eded252058e90e91f12f75f32aceae46ec9) Signed-off-by: Michael Tokarev --- nbd.c | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/nbd.c b/nbd.c index dc0adf9..6702b06 100644 --- a/nbd.c +++ b/nbd.c @@ -399,24 +399,23 @@ int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize) return -serrno; } -if (flags & NBD_FLAG_READ_ONLY) { -int read_only = 1; -TRACE("Setting readonly attribute"); - -if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) { +if (ioctl(fd, NBD_SET_FLAGS, flags) < 0) { +if (errno == ENOTTY) { +int read_only = (flags & NBD_FLAG_READ_ONLY) != 0; +TRACE("Setting readonly attribute"); + +if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) { +int serrno = errno; +LOG("Failed setting read-only attribute"); +return -serrno; +} +} else { int serrno = errno; -LOG("Failed setting read-only attribute"); +LOG("Failed setting flags"); return -serrno; } } -if (ioctl(fd, NBD_SET_FLAGS, flags) < 0 -&& errno != ENOTTY) { -int serrno = errno; -LOG("Failed setting flags"); -return -serrno; -} - TRACE("Negotiation ended"); return 0; -- 1.7.10.4
[Qemu-devel] [PATCH 10/60] pcie_aer: clear cmask for Advanced Error Interrupt Message Number
From: Jason Baron The Advanced Error Interrupt Message Number (bits 31:27 of the Root Error Status Register) is updated when the number of msi messages assigned to a device changes. Migration of windows 7 on q35 chipset failed because the check in get_pci_config_device() fails due to cmask being set on these bits. Its valid to update these bits and we must restore this state across migration. Signed-off-by: Jason Baron Signed-off-by: Michael S. Tsirkin (cherry picked from commit 0e180d9c8a7429c55d23d2e7855f1e490a063aaa) Signed-off-by: Michael Tokarev --- hw/pcie_aer.c |5 + 1 file changed, 5 insertions(+) diff --git a/hw/pcie_aer.c b/hw/pcie_aer.c index 3b6981c..b04c164 100644 --- a/hw/pcie_aer.c +++ b/hw/pcie_aer.c @@ -738,6 +738,11 @@ void pcie_aer_root_init(PCIDevice *dev) PCI_ERR_ROOT_CMD_EN_MASK); pci_set_long(dev->w1cmask + pos + PCI_ERR_ROOT_STATUS, PCI_ERR_ROOT_STATUS_REPORT_MASK); +/* PCI_ERR_ROOT_IRQ is RO but devices change it using a + * device-specific method. + */ +pci_set_long(dev->cmask + pos + PCI_ERR_ROOT_STATUS, + ~PCI_ERR_ROOT_IRQ); } void pcie_aer_root_reset(PCIDevice *dev) -- 1.7.10.4
Re: [Qemu-devel] [PATCH] get_tmp_filename: add explicit error message
On 02/04/2013 01:24 PM, Markus Armbruster wrote: > Fabien Chouteau writes: > >> On 02/04/2013 11:34 AM, Markus Armbruster wrote: >> >> Why? The caller doesn't know the difference between Windows/Linux >> implementation. And the error handling would have to be duplicated. > > The function's (implied) contract is to return an error code without > printing anything. If you want to change the contract to include > reporting the error, you need to implement it both for both arms of the > #ifdef. You also have to demonstrate that all callers are happy with > the change of contract. > > Regardless, printing to stderr is wrong. The function can be called on > behalf of a monitor command, and then the error needs to be printed to > the correct monitor. error_report() can do that for you, and more. > Alright, so I will use error_report() and do it for both Linux and Windows. >> It's not the first time I add error output in Windows code. Specially in >> block/, when there's an error, the only thing you get is: "operation not >> permitted". It's not very helpful and you have to dig in the code to >> find which function failed. > > Good error reporting is hard. Knowledge about the error and its context > gets lost as you move up the call chain. Knowledge about how to report > errors gets lost as you move down. > You're right, and in my opinion, no error reporting is the worst case. -- Fabien Chouteau
[Qemu-devel] [PATCH v3 03/10] main-loop: switch POSIX glib integration to GPollFD
Convert glib file descriptor polling from rfds/wfds/xfds to GPollFD. The Windows code still needs poll_fds[] and n_poll_fds but they can now become local variables. Signed-off-by: Stefan Hajnoczi --- main-loop.c | 71 +++-- 1 file changed, 22 insertions(+), 49 deletions(-) diff --git a/main-loop.c b/main-loop.c index f1dcd14..12b0213 100644 --- a/main-loop.c +++ b/main-loop.c @@ -145,8 +145,6 @@ int qemu_init_main_loop(void) static fd_set rfds, wfds, xfds; static int nfds; -static GPollFD poll_fds[1024 * 2]; /* this is probably overkill */ -static int n_poll_fds; static int max_priority; /* Load rfds/wfds/xfds into gpollfds. Will be removed a few commits later. */ @@ -206,65 +204,39 @@ static void gpollfds_to_select(int ret) } #ifndef _WIN32 -static void glib_select_fill(int *max_fd, fd_set *rfds, fd_set *wfds, - fd_set *xfds, uint32_t *cur_timeout) +static int glib_pollfds_idx; +static int glib_n_poll_fds; + +static void glib_pollfds_fill(uint32_t *cur_timeout) { GMainContext *context = g_main_context_default(); -int i; int timeout = 0; +int n; g_main_context_prepare(context, &max_priority); -n_poll_fds = g_main_context_query(context, max_priority, &timeout, - poll_fds, ARRAY_SIZE(poll_fds)); -g_assert(n_poll_fds <= ARRAY_SIZE(poll_fds)); - -for (i = 0; i < n_poll_fds; i++) { -GPollFD *p = &poll_fds[i]; - -if ((p->events & G_IO_IN)) { -FD_SET(p->fd, rfds); -*max_fd = MAX(*max_fd, p->fd); -} -if ((p->events & G_IO_OUT)) { -FD_SET(p->fd, wfds); -*max_fd = MAX(*max_fd, p->fd); -} -if ((p->events & G_IO_ERR)) { -FD_SET(p->fd, xfds); -*max_fd = MAX(*max_fd, p->fd); -} -} +glib_pollfds_idx = gpollfds->len; +n = glib_n_poll_fds; +do { +GPollFD *pfds; +glib_n_poll_fds = n; +g_array_set_size(gpollfds, glib_pollfds_idx + glib_n_poll_fds); +pfds = &g_array_index(gpollfds, GPollFD, glib_pollfds_idx); +n = g_main_context_query(context, max_priority, &timeout, pfds, + glib_n_poll_fds); +} while (n != glib_n_poll_fds); if (timeout >= 0 && timeout < *cur_timeout) { *cur_timeout = timeout; } } -static void glib_select_poll(fd_set *rfds, fd_set *wfds, fd_set *xfds, - bool err) +static void glib_pollfds_poll(void) { GMainContext *context = g_main_context_default(); +GPollFD *pfds = &g_array_index(gpollfds, GPollFD, glib_pollfds_idx); -if (!err) { -int i; - -for (i = 0; i < n_poll_fds; i++) { -GPollFD *p = &poll_fds[i]; - -if ((p->events & G_IO_IN) && FD_ISSET(p->fd, rfds)) { -p->revents |= G_IO_IN; -} -if ((p->events & G_IO_OUT) && FD_ISSET(p->fd, wfds)) { -p->revents |= G_IO_OUT; -} -if ((p->events & G_IO_ERR) && FD_ISSET(p->fd, xfds)) { -p->revents |= G_IO_ERR; -} -} -} - -if (g_main_context_check(context, max_priority, poll_fds, n_poll_fds)) { +if (g_main_context_check(context, max_priority, pfds, glib_n_poll_fds)) { g_main_context_dispatch(context); } } @@ -273,7 +245,7 @@ static int os_host_main_loop_wait(uint32_t timeout) { int ret; -glib_select_fill(&nfds, &rfds, &wfds, &xfds, &timeout); +glib_pollfds_fill(&timeout); if (timeout > 0) { qemu_mutex_unlock_iothread(); @@ -292,7 +264,7 @@ static int os_host_main_loop_wait(uint32_t timeout) qemu_mutex_lock_iothread(); } -glib_select_poll(&rfds, &wfds, &xfds, (ret < 0)); +glib_pollfds_poll(); return ret; } #else @@ -438,8 +410,9 @@ static void pollfds_poll(GArray *pollfds, int nfds, fd_set *rfds, static int os_host_main_loop_wait(uint32_t timeout) { GMainContext *context = g_main_context_default(); +GPollFD poll_fds[1024 * 2]; /* this is probably overkill */ int select_ret = 0; -int g_poll_ret, ret, i; +int g_poll_ret, ret, i, n_poll_fds; PollingEntry *pe; WaitObjects *w = &wait_objects; gint poll_timeout; -- 1.8.1
Re: [Qemu-devel] Question about default floppy and stdvga memory
Il 04/02/2013 11:36, Markus Armbruster ha scritto: Fabio Fantoni writes: I tried to disable "default floppy" without use -nodefaults option that disable other things. I didn't found other parameters to do that in docs and code for now. Can someone tell me if there is another way to disable default floppy only? As far as I know, there is none. Can someone add an option (for example -nofloppy) for disable default floppy? [...] smime.p7s Description: Firma crittografica S/MIME
[Qemu-devel] [PATCH 29/60] usb-storage: fix SYNCHRONIZE_CACHE
From: Gerd Hoffmann Commit 59310659073d85745854f2f10c4292555c5a1c51 is incomplete, we'll arrive in the scsi command complete callback in CSW state and must handle that case correctly. Signed-off-by: Gerd Hoffmann (cherry picked from commit 54414218d78c9d043417b27bb29bd0334b4e3cb5) Signed-off-by: Michael Tokarev --- hw/usb/dev-storage.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index ae22fb1..3a993d3 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -242,6 +242,9 @@ static void usb_msd_command_complete(SCSIRequest *req, uint32_t status, size_t r the status read packet. */ usb_msd_send_status(s, p); s->mode = USB_MSDM_CBW; +} else if (s->mode == USB_MSDM_CSW) { +usb_msd_send_status(s, p); +s->mode = USB_MSDM_CBW; } else { if (s->data_len) { int len = (p->iov.size - p->result); -- 1.7.10.4
[Qemu-devel] [PATCH 36/60] PPC: Bamboo: Fix memory size DT property
From: Alexander Graf Device tree properties need to be specified in big endian. Fix the bamboo memory size property accordingly. Signed-off-by: Alexander Graf CC: qemu-sta...@nongnu.org (cherry picked from commit 5232fa59b17b45c04bd24e0d38224964816bf391) Signed-off-by: Michael Tokarev --- hw/ppc440_bamboo.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ppc440_bamboo.c b/hw/ppc440_bamboo.c index f0a3ae4..0833228 100644 --- a/hw/ppc440_bamboo.c +++ b/hw/ppc440_bamboo.c @@ -59,7 +59,7 @@ static int bamboo_load_device_tree(target_phys_addr_t addr, { int ret = -1; #ifdef CONFIG_FDT -uint32_t mem_reg_property[] = { 0, 0, ramsize }; +uint32_t mem_reg_property[] = { 0, 0, cpu_to_be32(ramsize) }; char *filename; int fdt_size; void *fdt; -- 1.7.10.4
[Qemu-devel] [PATCH 24/60] qxl: always update displaysurface on resize
From: Gerd Hoffmann Don't try to be clever and skip displaysurface reinitialization in case the size hasn't changed. Other parameters might have changed nevertheless, for example depth or stride, resulting in rendering being broken then. Trigger: boot linux guest with vesafb, start X11, make sure both vesafb and X11 use the display same resolution. Then watch X11 screen being upside down. Signed-off-by: Gerd Hoffmann (cherry picked from commit 0ec8df3974d2a4ff95b5fd4785b9bd3def7252f3) Signed-off-by: Michael Tokarev --- hw/qxl-render.c |4 1 file changed, 4 deletions(-) diff --git a/hw/qxl-render.c b/hw/qxl-render.c index e2e3fe2..b66c168 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -99,7 +99,6 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl) { VGACommonState *vga = &qxl->vga; int i; -DisplaySurface *surface = vga->ds->surface; if (qxl->guest_primary.resized) { qxl->guest_primary.resized = 0; @@ -112,9 +111,6 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl) qxl->guest_primary.qxl_stride, qxl->guest_primary.bytes_pp, qxl->guest_primary.bits_pp); -} -if (surface->width != qxl->guest_primary.surface.width || -surface->height != qxl->guest_primary.surface.height) { if (qxl->guest_primary.qxl_stride > 0) { qemu_free_displaysurface(vga->ds); qemu_create_displaysurface_from(qxl->guest_primary.surface.width, -- 1.7.10.4
Re: [Qemu-devel] [PATCH qom-cpu-next v3 3/4] target-i386: Slim conversion to X86CPU subclasses
On Sat, 2 Feb 2013 01:37:07 +0100 Andreas Färber wrote: > Move x86_def_t definition to header and embed into X86CPUClass. > Register types per built-in model definition. > > Move version initialization from x86_cpudef_setup() to class_init. > > Inline cpu_x86_register() into the X86CPU initfn. > Since instance_init cannot reports errors, drop error handling. > > Replace cpu_x86_find_by_name() with x86_cpu_class_by_name(). > Move KVM host vendor override from cpu_x86_find_by_name() to the initfn. > > Register host-{i386,x86_64}-cpu type from KVM code to avoid #ifdefs. > Make kvm_cpu_fill_host() a class_init and inline cpu_x86_fill_model_id(). > > Let kvm_check_features_against_host() obtain host-{i386,86_64}-cpu for > comparison. > > Signed-off-by: Andreas Färber > --- > target-i386/cpu-qom.h | 24 > target-i386/cpu.c | 324 > + target-i386/cpu.h > |2 - target-i386/kvm.c | 93 ++ > 4 Dateien geändert, 228 Zeilen hinzugefügt(+), 215 Zeilen entfernt(-) > > diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h > index 48e6b54..80bf72d 100644 > --- a/target-i386/cpu-qom.h > +++ b/target-i386/cpu-qom.h > @@ -30,6 +30,27 @@ > #define TYPE_X86_CPU "i386-cpu" > #endif > > +#define TYPE_HOST_X86_CPU "host-" TYPE_X86_CPU > + > +typedef struct x86_def_t { > +const char *name; > +uint32_t level; > +/* vendor is zero-terminated, 12 character ASCII string */ > +char vendor[CPUID_VENDOR_SZ + 1]; > +int family; > +int model; > +int stepping; > +uint32_t features, ext_features, ext2_features, ext3_features; > +uint32_t kvm_features, svm_features; > +uint32_t xlevel; > +char model_id[48]; > +/* Store the results of Centaur's CPUID instructions */ > +uint32_t ext4_features; > +uint32_t xlevel2; > +/* The feature bits on CPUID[EAX=7,ECX=0].EBX */ > +uint32_t cpuid_7_0_ebx_features; > +} x86_def_t; > + > #define X86_CPU_CLASS(klass) \ > OBJECT_CLASS_CHECK(X86CPUClass, (klass), TYPE_X86_CPU) > #define X86_CPU(obj) \ > @@ -41,6 +62,7 @@ > * X86CPUClass: > * @parent_realize: The parent class' realize handler. > * @parent_reset: The parent class' reset handler. > + * @info: Model-specific data. > * > * An x86 CPU model or family. > */ > @@ -51,6 +73,8 @@ typedef struct X86CPUClass { > > DeviceRealize parent_realize; > void (*parent_reset)(CPUState *cpu); > + > +x86_def_t info; Is it necessary to embed it here, could pointer to corresponding static array be used here? That way one could avoid extra memcpy in class_init(). > } X86CPUClass; > > /** > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index ee2fd6b..6c95740 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -346,25 +346,6 @@ static void add_flagname_to_bitmaps(const char > *flagname, } > } > > -typedef struct x86_def_t { > -const char *name; > -uint32_t level; > -/* vendor is zero-terminated, 12 character ASCII string */ > -char vendor[CPUID_VENDOR_SZ + 1]; > -int family; > -int model; > -int stepping; > -uint32_t features, ext_features, ext2_features, ext3_features; > -uint32_t kvm_features, svm_features; > -uint32_t xlevel; > -char model_id[48]; > -/* Store the results of Centaur's CPUID instructions */ > -uint32_t ext4_features; > -uint32_t xlevel2; > -/* The feature bits on CPUID[EAX=7,ECX=0].EBX */ > -uint32_t cpuid_7_0_ebx_features; > -} x86_def_t; > - > #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) > #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \ >CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC) > @@ -868,86 +849,6 @@ static x86_def_t builtin_x86_defs[] = { > }, > }; > > -#ifdef CONFIG_KVM > -static int cpu_x86_fill_model_id(char *str) > -{ > -uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; > -int i; > - > -for (i = 0; i < 3; i++) { > -host_cpuid(0x8002 + i, 0, &eax, &ebx, &ecx, &edx); > -memcpy(str + i * 16 + 0, &eax, 4); > -memcpy(str + i * 16 + 4, &ebx, 4); > -memcpy(str + i * 16 + 8, &ecx, 4); > -memcpy(str + i * 16 + 12, &edx, 4); > -} > -return 0; > -} > -#endif > - > -/* Fill a x86_def_t struct with information about the host CPU, and > - * the CPU features supported by the host hardware + host kernel > - * > - * This function may be called only if KVM is enabled. > - */ > -static void kvm_cpu_fill_host(x86_def_t *x86_cpu_def) > -{ > -#ifdef CONFIG_KVM > -KVMState *s = kvm_state; > -uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; > - > -assert(kvm_enabled()); > - > -x86_cpu_def->name = "host"; > -host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); > -x86_cpu_vendor_words2str(x86_cpu_def->vendor, ebx, edx, ecx); > - > -host_cpuid(0x1, 0, &eax, &ebx, &ecx, &edx); > -x86_cpu_def->family = ((eax >> 8) & 0x0F) + ((eax >> 20) & 0x
Re: [Qemu-devel] [PATCH for-1.4] target-cris: Build fix for debug output
Am 27.01.2013 07:26, schrieb Andreas Färber: > Around r3361 (81fdc5f8d2d681da8d255baf0713144f8656bac9) env->debug1 used > to contain the address of an MMU fault. This is now written into > env->pregs[PR_EDA] instead. > > Signed-off-by: Andreas Färber > --- > target-cris/op_helper.c |2 +- > 1 Datei geändert, 1 Zeile hinzugefügt(+), 1 Zeile entfernt(-) Ping! Resent as part of my debug output RFC already, and it missed rc0. CC'ing qemu-trivial as it should be easily confirmable by comparing the mentioned commit with today's MMU fault handler (helper.c iirc). Andreas > diff --git a/target-cris/op_helper.c b/target-cris/op_helper.c > index 0f6a1ee..b580513 100644 > --- a/target-cris/op_helper.c > +++ b/target-cris/op_helper.c > @@ -60,7 +60,7 @@ void tlb_fill(CPUCRISState *env, target_ulong addr, int > is_write, int mmu_idx, > int ret; > > D_LOG("%s pc=%x tpc=%x ra=%p\n", __func__, > - env->pc, env->debug1, (void *)retaddr); > + env->pc, env->pregs[PR_EDA], (void *)retaddr); > ret = cpu_cris_handle_mmu_fault(env, addr, is_write, mmu_idx); > if (unlikely(ret)) { > if (retaddr) { -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH] get_tmp_filename: add explicit error message
Am 04.02.2013 14:33, schrieb Fabien Chouteau: > On 02/04/2013 01:24 PM, Markus Armbruster wrote: >> >> Good error reporting is hard. Knowledge about the error and its context >> gets lost as you move up the call chain. Knowledge about how to report >> errors gets lost as you move down. >> > > You're right, and in my opinion, no error reporting is the worst case. The best solution would be to pass an Error **errp argument and use error_setg() on it, as done for visitors and QOM. Then the caller can decide whether to pass NULL and the callee can report detailed errors. Invasive change obviously and thus not suitable for 1.4. Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
[Qemu-devel] [PATCH] block/raw-posix: detect readonly LVM volumes using BLKROGET
LVM volumes can be set read-only with "lvchange --permission r ". The device node permissions remain unchanged so the volume can still be opened O_RDWR. Actual writes will fail. This results in odd behavior for QEMU. bdrv_open() is supposed to fail if a read-only image is being opened with BDRV_O_RDWR. By not failing for LVM volumes, the guest boots up but every write produces an I/O error. This patch checks whether the block device is read-only so that LVM volumes behave like regular files. Reported-by: Sibiao Luo Suggested-by: Paolo Bonzini Signed-off-by: Stefan Hajnoczi --- block/raw-posix.c | 48 +++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/block/raw-posix.c b/block/raw-posix.c index 8b6b926..2ab4da4 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -1257,9 +1257,42 @@ static int hdev_probe_device(const char *filename) return 0; } +static int check_hdev_writable(BDRVRawState *s) +{ +#if defined(BLKROGET) +/* Linux LVM volumes can be configured "read-only" using lvchange(8). This + * does not change permissions on the device node and therefore open(2) + * with O_RDWR succeeds. Actual writes fail with EPERM. + * + * bdrv_open() is supposed to fail if the disk is read-only. Explicitly + * check for read-only block devices so that LVM volumes behave properly. + */ +struct stat st; +int readonly = 0; + +if (fstat(s->fd, &st)) { +return -errno; +} + +if (!S_ISBLK(st.st_mode)) { +return 0; +} + +if (ioctl(s->fd, BLKROGET, &readonly) < 0) { +return -errno; +} + +if (readonly) { +return -EACCES; +} +#endif /* defined(BLKROGET) */ +return 0; +} + static int hdev_open(BlockDriverState *bs, const char *filename, int flags) { BDRVRawState *s = bs->opaque; +int ret; #if defined(__APPLE__) && defined(__MACH__) if (strstart(filename, "/dev/cdrom", NULL)) { @@ -1300,7 +1333,20 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags) } #endif -return raw_open_common(bs, filename, flags, 0); +ret = raw_open_common(bs, filename, flags, 0); +if (ret < 0) { +return ret; +} + +if (flags & BDRV_O_RDWR) { +ret = check_hdev_writable(s); +if (ret < 0) { +raw_close(bs); +return ret; +} +} + +return ret; } #if defined(__linux__) -- 1.8.1
[Qemu-devel] [PATCH 51/60] vmdk: Fix data corruption bug in WRITE and READ handling
From: Gerhard Wiesinger Fixed a MAJOR BUG in VMDK files on file boundaries on reads and ALSO ON WRITES WHICH MIGHT CORRUPT THE IMAGE AND DATA!! Triggered for example with the following VMDK file (partly listed): RW 4193792 FLAT "XP-W1-f001.vmdk" 0 RW 2097664 FLAT "XP-W1-f002.vmdk" 0 RW 4193792 FLAT "XP-W1-f003.vmdk" 0 RW 512 FLAT "XP-W1-f004.vmdk" 0 RW 4193792 FLAT "XP-W1-f005.vmdk" 0 RW 2097664 FLAT "XP-W1-f006.vmdk" 0 RW 4193792 FLAT "XP-W1-f007.vmdk" 0 RW 512 FLAT "XP-W1-f008.vmdk" 0 Patch includes: 1.) Patch fixes wrong calculation on extent boundaries. Especially it fixes the relativeness of the sector number to the current extent. Verfied correctness with: 1.) Converted either with Virtualbox to VDI and then with qemu-img and then with qemu-img only: VBoxManage clonehd --format vdi /VM/XP-W/new/XP-W1.vmdk ~/.VirtualBox/Harddisks/XP-W1-new-test.vdi ./qemu-img convert -O raw ~/.VirtualBox/Harddisks/XP-W1-new-test.vdi /root/QEMU/VM-XP-W1/XP-W1-via-VBOX.img md5sum /root/QEMU/VM-XP-W/XP-W1-direct.img md5sum /root/QEMU/VM-XP-W/XP-W1-via-VBOX.img => same MD5 hash 2.) Verified debug log files 3.) Run Windows XP successfully 4.) chkdsk run successfully without any errors Signed-off-by: Gerhard Wiesinger Acked-by: Fam Zheng Signed-off-by: Kevin Wolf (cherry picked from commit b1649fae49a899a222c3ac53c5009dd6f23349e1) Signed-off-by: Michael Tokarev --- block/vmdk.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index a55f756..f0448ba 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1058,6 +1058,7 @@ static int vmdk_read(BlockDriverState *bs, int64_t sector_num, BDRVVmdkState *s = bs->opaque; int ret; uint64_t n, index_in_cluster; +uint64_t extent_begin_sector, extent_relative_sector_num; VmdkExtent *extent = NULL; uint64_t cluster_offset; @@ -1069,7 +1070,9 @@ static int vmdk_read(BlockDriverState *bs, int64_t sector_num, ret = get_cluster_offset( bs, extent, NULL, sector_num << 9, 0, &cluster_offset); -index_in_cluster = sector_num % extent->cluster_sectors; +extent_begin_sector = extent->end_sector - extent->sectors; +extent_relative_sector_num = sector_num - extent_begin_sector; +index_in_cluster = extent_relative_sector_num % extent->cluster_sectors; n = extent->cluster_sectors - index_in_cluster; if (n > nb_sectors) { n = nb_sectors; @@ -1120,6 +1123,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num, VmdkExtent *extent = NULL; int n, ret; int64_t index_in_cluster; +uint64_t extent_begin_sector, extent_relative_sector_num; uint64_t cluster_offset; VmdkMetaData m_data; @@ -1162,7 +1166,9 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num, if (ret) { return -EINVAL; } -index_in_cluster = sector_num % extent->cluster_sectors; +extent_begin_sector = extent->end_sector - extent->sectors; +extent_relative_sector_num = sector_num - extent_begin_sector; +index_in_cluster = extent_relative_sector_num % extent->cluster_sectors; n = extent->cluster_sectors - index_in_cluster; if (n > nb_sectors) { n = nb_sectors; -- 1.7.10.4
[Qemu-devel] [PATCH 01/60] tcg/s390: fix ld/st with CONFIG_TCG_PASS_AREG0
From: Aurelien Jarno The load/store slow path has been broken in e141ab52d: - We need to move 4 registers for store functions and 3 registers for load functions and not the reverse. - According to the s390x calling convention the arguments of a function should be zero extended. This means that the register shift should be done with TCG_TYPE_I64 to ensure the higher word is correctly zero extended when needed. I am aware that CONFIG_TCG_PASS_AREG0 is being removed and thus that this patch can be improved, but doing so means it can also be applied to the 1.1 and 1.2 stable branches. Signed-off-by: Aurelien Jarno Signed-off-by: Alexander Graf (cherry picked from commit 6845df48cec9cc6833429942b3ceed333a791119) Signed-off-by: Michael Tokarev --- tcg/s390/tcg-target.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c index 04662c1..99b5339 100644 --- a/tcg/s390/tcg-target.c +++ b/tcg/s390/tcg-target.c @@ -1509,11 +1509,13 @@ static void tcg_prepare_qemu_ldst(TCGContext* s, TCGReg data_reg, tcg_out_movi(s, TCG_TYPE_I32, TCG_REG_R4, mem_index); #ifdef CONFIG_TCG_PASS_AREG0 /* XXX/FIXME: suboptimal */ -tcg_out_mov(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[2], +tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[3], +tcg_target_call_iarg_regs[2]); +tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2], tcg_target_call_iarg_regs[1]); -tcg_out_mov(s, TCG_TYPE_TL, tcg_target_call_iarg_regs[1], +tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[1], tcg_target_call_iarg_regs[0]); -tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0], +tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[0], TCG_AREG0); #endif tgen_calli(s, (tcg_target_ulong)qemu_st_helpers[s_bits]); @@ -1521,13 +1523,11 @@ static void tcg_prepare_qemu_ldst(TCGContext* s, TCGReg data_reg, tcg_out_movi(s, TCG_TYPE_I32, arg1, mem_index); #ifdef CONFIG_TCG_PASS_AREG0 /* XXX/FIXME: suboptimal */ -tcg_out_mov(s, TCG_TYPE_I32, tcg_target_call_iarg_regs[3], -tcg_target_call_iarg_regs[2]); tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[2], tcg_target_call_iarg_regs[1]); -tcg_out_mov(s, TCG_TYPE_TL, tcg_target_call_iarg_regs[1], +tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[1], tcg_target_call_iarg_regs[0]); -tcg_out_mov(s, TCG_TYPE_PTR, tcg_target_call_iarg_regs[0], +tcg_out_mov(s, TCG_TYPE_I64, tcg_target_call_iarg_regs[0], TCG_AREG0); #endif tgen_calli(s, (tcg_target_ulong)qemu_ld_helpers[s_bits]); -- 1.7.10.4
[Qemu-devel] [PATCH 06/10] qdev: Implement (variable length) array properties
Add support for declaring array properties for qdev devices. These work by defining an initial static property 'len-arrayname' which the user of the device should set to the desired size of the array. When this property is set, memory is allocated for the array elements, and dynamic properties "arrayname[0]", "arrayname[1]"... are created so the user of the device can then set the values of the individual array elements. Signed-off-by: Peter Maydell --- hw/qdev-core.h |3 ++ hw/qdev-properties.c | 104 ++ hw/qdev-properties.h | 39 +++ 3 files changed, 146 insertions(+) diff --git a/hw/qdev-core.h b/hw/qdev-core.h index 2486f36..547fbc7 100644 --- a/hw/qdev-core.h +++ b/hw/qdev-core.h @@ -175,6 +175,9 @@ struct Property { uint8_t bitnr; uint8_t qtype; int64_t defval; +int arrayoffset; +PropertyInfo *arrayinfo; +int arrayfieldsize; }; struct PropertyInfo { diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c index a8a31f5..5f03942 100644 --- a/hw/qdev-properties.c +++ b/hw/qdev-properties.c @@ -779,6 +779,110 @@ PropertyInfo qdev_prop_pci_host_devaddr = { .set = set_pci_host_devaddr, }; +/* --- support for array properties --- */ + +/* Used as an opaque for the object properties we add for each + * array element. Note that the struct Property must be first + * in the struct so that a pointer to this works as the opaque + * for the underlying element's property hooks as well as for + * our own release callback. + */ +typedef struct { +struct Property prop; +char *propname; +ObjectPropertyRelease *release; +} ArrayElementProperty; + +/* object property release callback for array element properties: + * we call the underlying element's property release hook, and + * then free the memory we allocated when we added the property. + */ +static void array_element_release(Object *obj, const char *name, void *opaque) +{ +ArrayElementProperty *p = opaque; +if (p->release) { +p->release(obj, name, opaque); +} +g_free(p->propname); +g_free(p); +} + +static void set_prop_arraylen(Object *obj, Visitor *v, void *opaque, + const char *name, Error **errp) +{ +/* Setter for the property which defines the length of a + * variable-sized property array. As well as actually setting the + * array-length field in the device struct, we have to create the + * array itself and dynamically add the corresponding properties. + */ +DeviceState *dev = DEVICE(obj); +Property *prop = opaque; +uint32_t *alenptr = qdev_get_prop_ptr(dev, prop); +void **arrayptr = (void *)dev + prop->arrayoffset; +void *eltptr; +const char *arrayname; +int i; + +if (dev->realized) { +error_set(errp, QERR_PERMISSION_DENIED); +return; +} +if (*alenptr) { +error_setg(errp, "array size property %s may not be set more than once", + name); +return; +} +visit_type_uint32(v, alenptr, name, errp); +if (error_is_set(errp)) { +return; +} +if (!*alenptr) { +return; +} + +/* DEFINE_PROP_ARRAY guarantees that name should start with this prefix; + * strip it off so we can get the name of the array itself. + */ +assert(strncmp(name, PROP_ARRAY_LEN_PREFIX, + strlen(PROP_ARRAY_LEN_PREFIX)) == 0); +arrayname = name + strlen(PROP_ARRAY_LEN_PREFIX); + +/* Note that it is the responsibility of the individual device's deinit + * to free the array proper. + */ +*arrayptr = eltptr = g_malloc0(*alenptr * prop->arrayfieldsize); +for (i = 0; i < *alenptr; i++, eltptr += prop->arrayfieldsize) { +char *propname = g_strdup_printf("%s[%d]", arrayname, i); +ArrayElementProperty *arrayprop = g_new0(ArrayElementProperty, 1); +arrayprop->release = prop->arrayinfo->release; +arrayprop->propname = propname; +arrayprop->prop.info = prop->arrayinfo; +arrayprop->prop.name = propname; +/* This ugly piece of pointer arithmetic sets up the offset so + * that when the underlying get/set hooks call qdev_get_prop_ptr + * they get the right answer despite the array element not actually + * being inside the device struct. + */ +arrayprop->prop.offset = eltptr - (void *)dev; +assert(qdev_get_prop_ptr(dev, &arrayprop->prop) == eltptr); +object_property_add(obj, propname, +arrayprop->prop.info->name, +arrayprop->prop.info->get, +arrayprop->prop.info->set, +array_element_release, +arrayprop, errp); +if (error_is_set(errp)) { +return; +} +} +} + +PropertyInfo qdev_prop_arraylen = { +.name = "uint32", +
[Qemu-devel] [PATCH v3 05/10] slirp: switch to GPollFD
Slirp uses rfds/wfds/xfds more extensively than other QEMU components. The rarely-used out-of-band TCP data feature is used. That means we need the full table of select(2) to g_poll(3) events: rfds -> G_IO_IN | G_IO_HUP | G_IO_ERR wfds -> G_IO_OUT | G_IO_ERR xfds -> G_IO_PRI I came up with this table by looking at Linux fs/select.c which maps select(2) to poll(2) internally. Another detail to watch out for are the global variables that reference rfds/wfds/xfds during slirp_select_poll(). sofcantrcvmore() and sofcantsendmore() use these globals to clear fd_set bits. When sofcantrcvmore() is called, the wfds bit is cleared so that the write handler will no longer be run for this iteration of the event loop. This actually seems buggy to me since TCP connections can be half-closed and we'd still want to handle data in half-duplex fashion. I think the real intention is to avoid running the read/write handler when the socket has been fully closed. This is indicated with the SS_NOFDREF state bit so we now check for it before invoking the TCP write handler. Note that UDP/ICMP code paths don't care because they are connectionless. Note that slirp/ has a lot of tabs and sometimes mixed tabs with spaces. I followed the style of the surrounding code. Signed-off-by: Stefan Hajnoczi --- main-loop.c | 4 +- slirp/libslirp.h | 6 +-- slirp/main.h | 1 - slirp/slirp.c| 133 +-- slirp/socket.c | 9 slirp/socket.h | 2 + stubs/slirp.c| 6 +-- 7 files changed, 87 insertions(+), 74 deletions(-) diff --git a/main-loop.c b/main-loop.c index 12b0213..49e97ff 100644 --- a/main-loop.c +++ b/main-loop.c @@ -503,13 +503,13 @@ int main_loop_wait(int nonblocking) #ifdef CONFIG_SLIRP slirp_update_timeout(&timeout); -slirp_select_fill(&nfds, &rfds, &wfds, &xfds); +slirp_pollfds_fill(gpollfds); #endif qemu_iohandler_fill(&nfds, &rfds, &wfds, &xfds); ret = os_host_main_loop_wait(timeout); qemu_iohandler_poll(&rfds, &wfds, &xfds, ret); #ifdef CONFIG_SLIRP -slirp_select_poll(&rfds, &wfds, &xfds, (ret < 0)); +slirp_pollfds_poll(gpollfds, (ret < 0)); #endif qemu_run_all_timers(); diff --git a/slirp/libslirp.h b/slirp/libslirp.h index 49609c2..ceabff8 100644 --- a/slirp/libslirp.h +++ b/slirp/libslirp.h @@ -17,11 +17,9 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork, void slirp_cleanup(Slirp *slirp); void slirp_update_timeout(uint32_t *timeout); -void slirp_select_fill(int *pnfds, - fd_set *readfds, fd_set *writefds, fd_set *xfds); +void slirp_pollfds_fill(GArray *pollfds); -void slirp_select_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, - int select_error); +void slirp_pollfds_poll(GArray *pollfds, int select_error); void slirp_input(Slirp *slirp, const uint8_t *pkt, int pkt_len); diff --git a/slirp/main.h b/slirp/main.h index 66e4f92..f2e58cf 100644 --- a/slirp/main.h +++ b/slirp/main.h @@ -31,7 +31,6 @@ extern int ctty_closed; extern char *slirp_tty; extern char *exec_shell; extern u_int curtime; -extern fd_set *global_readfds, *global_writefds, *global_xfds; extern struct in_addr loopback_addr; extern unsigned long loopback_mask; extern char *username; diff --git a/slirp/slirp.c b/slirp/slirp.c index 5d14e7f..bd9b7cb 100644 --- a/slirp/slirp.c +++ b/slirp/slirp.c @@ -39,9 +39,6 @@ static const uint8_t special_ethaddr[ETH_ALEN] = { static const uint8_t zero_ethaddr[ETH_ALEN] = { 0, 0, 0, 0, 0, 0 }; -/* XXX: suppress those select globals */ -fd_set *global_readfds, *global_writefds, *global_xfds; - u_int curtime; static u_int time_fasttimo, last_slowtimo; static int do_slowtimo; @@ -261,7 +258,6 @@ void slirp_cleanup(Slirp *slirp) #define CONN_CANFSEND(so) (((so)->so_state & (SS_FCANTSENDMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED) #define CONN_CANFRCV(so) (((so)->so_state & (SS_FCANTRCVMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED) -#define UPD_NFDS(x) if (nfds < (x)) nfds = (x) void slirp_update_timeout(uint32_t *timeout) { @@ -270,23 +266,15 @@ void slirp_update_timeout(uint32_t *timeout) } } -void slirp_select_fill(int *pnfds, - fd_set *readfds, fd_set *writefds, fd_set *xfds) +void slirp_pollfds_fill(GArray *pollfds) { Slirp *slirp; struct socket *so, *so_next; -int nfds; if (QTAILQ_EMPTY(&slirp_instances)) { return; } -/* fail safe */ -global_readfds = NULL; -global_writefds = NULL; -global_xfds = NULL; - -nfds = *pnfds; /* * First, TCP sockets */ @@ -302,8 +290,12 @@ void slirp_select_fill(int *pnfds, for (so = slirp->tcb.so_next; so != &slirp->tcb; so = so_next) { +int events = 0; + so_next = so->so_next; +so->pollfds_idx = -1; + /* * See if we need a tcp_fasttimo */ @@ -323,8
[Qemu-devel] [PATCH 37/60] target-sparc64: disable VGA cirrus
From: Aurelien Jarno OpenBIOS on sparc64 only support Standard VGA and not Cirrus VGA. Don't build Cirrus VGA support so that it can't be selected. This fixes the breakage introduced by commit f2898771. Reported-by: Richard Henderson Cc: Blue Swirl Signed-off-by: Aurelien Jarno Tested-by: Richard Henderson Signed-off-by: Blue Swirl (cherry picked from commit 0356404b0f1da939657cad1efeb556745cd430d5) Signed-off-by: Michael Tokarev --- default-configs/sparc64-softmmu.mak |1 - 1 file changed, 1 deletion(-) diff --git a/default-configs/sparc64-softmmu.mak b/default-configs/sparc64-softmmu.mak index c9a36c1..03e8b42 100644 --- a/default-configs/sparc64-softmmu.mak +++ b/default-configs/sparc64-softmmu.mak @@ -6,7 +6,6 @@ CONFIG_M48T59=y CONFIG_PTIMER=y CONFIG_VGA=y CONFIG_VGA_PCI=y -CONFIG_VGA_CIRRUS=y CONFIG_SERIAL=y CONFIG_PARALLEL=y CONFIG_PCKBD=y -- 1.7.10.4
Re: [Qemu-devel] Headsup: windows virtio networking does not work on current git
Rusty Russell writes: > Anthony Liguori writes: >> Michael Tokarev writes: >> >>> 03.02.2013 17:23, Yan Vugenfirer wrote: >>> > If it helps, mq changes the config size from 8 to 16 bytes. If the > driver was making an assumption about an 8-byte config size, that's > likely what the problem is. > That's exactly the problem. > ... >> N.B. this is a pretty nasty bug in the guest driver. Any new virtio-net >> feature is going to trigger it (not just multiqueue). So while pc-1.3 >> will work forever with this guest image, it's pretty much guaranteed to >> break at some point in the future. > > Wow, yes. I never expected such a bug. I probably don't even want to > know how this happened, right? > > If I could find a way, I'd like to create some code as an appendix to > the virtio spec which would torture test each driver and/or device by > configuring it in strange ways. But that's pure speculation at this > point. It wouldn't be so hard to add a torture parameter to the virtio implementation in QEMU that would do silly things that were still within the bounds of the specification. Larger config sizes, advertisement of unknown feature bits, etc. >>> It's easy to turn off mq by default and turn it on when >>> needed... >>> >>> The problem now is that it isn't obvious why your existing >>> setup breaks when you upgrade. While when you especially >>> play with mq, you may look at options available around it... >> >> If we ever to get virtio2, a really handy feature to have would be a >> driver identification string. Even if was only informative (and not >> authoritative, we've had a lot of issues like this and being able to >> make work arounds conditional on the driver identification string would >> be nice. > > In practice, we'd want to formalize it into a string and a version, and > hope the version gets bumped appropriately. Because it'll actually be > used for bug detection. Sure. > But AFAICT that would be useless in this case. We really want to know about > the guest before we even start it. We can't just prepare to fight yesterday's wars :-) We'd want to know the string before we expose host features. That's easy. We've had a lot of trouble with certain feature bits breaking drivers so masking certain features for known broken drivers would be really useful. How we expose config space in virtio2 is a separate discussion. If we take a list approach like you've talked about before, it would be much harder for drivers to assume anything about the BAR size for config since the size would always be different. Regards, Anthony Liguori > > Cheers, > Rusty.
Re: [Qemu-devel] [PATCH v3 02/10] main-loop: switch to g_poll() on POSIX hosts
Comments in-line. On 02/04/13 13:12, Stefan Hajnoczi wrote: > Use g_poll(3) instead of select(2). Well, this is kind of a cheat. > It's true that we're now using g_poll(3) on POSIX hosts but the *_fill() > and *_poll() functions are still using rfds/wfds/xfds. > > We've set the scene to start converting *_fill() and *_poll() functions > step-by-step until no more rfds/wfds/xfds users remain. Then we'll drop > the temporary gpollfds_from_select() and gpollfds_to_select() functions > and be left with native g_poll(2). > > On Windows things are a little crazy: convert from rfds/wfds/xfds to > GPollFDs, back to rfds/wfds/xfds, call select(2), rfds/wfds/xfds back to > GPollFDs, and finally back to rfds/wfds/xfds again. This is only > temporary and keeps the Windows build working through the following > patches. We'll drop this excessive conversion later and be left with a > single GPollFDs -> select(2) -> GPollFDs sequence that allows Windows to > use select(2) while the rest of QEMU only knows about GPollFD. > > Signed-off-by: Stefan Hajnoczi > --- > main-loop.c | 135 > > 1 file changed, 127 insertions(+), 8 deletions(-) I assume this is complex because the pre-patch situation is overly complex as well: slirp -> fd_set iohandler -> fd_set unix windows - beforeafter beforeafter -- - - - glib -> fd_set glib -> fd_set glib-> GPollFD glib-> GPollFD fd_set -> GPollFD WaitObj -> GPollFD WaitObj -> GPollFD SELECT G_POLL G_POLL (glib/WaitObj) G_POLL (glib/WaitObj) fd_set -> GPollFD GPollFD -> fd_set SELECT (slirp/ioh.)SELECT (slirp/ioh.) (I'm ignoring the after-select / after-poll operations; they (should) mirror the pre-select / pre-poll ones.) No idea why the windows version has been mixing g_poll() and select(). I'd hope that this series kills select() for uniformity's sake, but the 00/10 subject and the commit msg for this patch indicate otherwise. "main-loop.c" is full of global variables which makes it a *pain* to read. > > diff --git a/main-loop.c b/main-loop.c > index d0d8fe4..f1dcd14 100644 > --- a/main-loop.c > +++ b/main-loop.c > @@ -117,6 +117,8 @@ void qemu_notify_event(void) > aio_notify(qemu_aio_context); > } > > +static GArray *gpollfds; > + > int qemu_init_main_loop(void) > { > int ret; > @@ -133,6 +135,7 @@ int qemu_init_main_loop(void) > return ret; > } > > +gpollfds = g_array_new(FALSE, FALSE, sizeof(GPollFD)); > qemu_aio_context = aio_context_new(); > src = aio_get_g_source(qemu_aio_context); > g_source_attach(src, NULL); > @@ -146,6 +149,62 @@ static GPollFD poll_fds[1024 * 2]; /* this is probably > overkill */ > static int n_poll_fds; > static int max_priority; > > +/* Load rfds/wfds/xfds into gpollfds. Will be removed a few commits later. > */ > +static void gpollfds_from_select(void) > +{ > +int fd; > +for (fd = 0; fd <= nfds; fd++) { "nfds" is an example of a global variable being global ("file scope") for no good reason. Anyway it does seem to have inclusive meaning ("highest file descriptor in all sets"). > +int events = 0; > +if (FD_ISSET(fd, &rfds)) { > +events |= G_IO_IN | G_IO_HUP | G_IO_ERR; > +} ("=" would have sufficed) > +if (FD_ISSET(fd, &wfds)) { > +events |= G_IO_OUT | G_IO_ERR; > +} > +if (FD_ISSET(fd, &xfds)) { > +events |= G_IO_PRI; > +} > +if (events) { > +GPollFD pfd = { > +.fd = fd, > +.events = events, > +}; > +g_array_append_val(gpollfds, pfd); > +} > +} > +} (I don't like "gpollfds" being global, but) this seems OK. It matches the glib docs and also How the Mapping Should Be Done (TM). This function corresponds to the "unix / after / fd_set -> GPollFD" entry of the diagram, and as such it is composed with "glib -> fd_set" (glib_select_fill()). I'll note that glib_select_fill() sets "xfds" for G_IO_ERR but not G_IO_PRI. (According to SUSv4, in our case xfds is the logical-or of "error pending" and "OOB/urgent pending".) We can assume that each entry stored by g_main_context_query() will have at least one of IN and OUT set in "events". Further assuming that glib follows its own documentation, that implies that G_IO_ERR will be set unconditionally (because it always accompanies each of IN and OUT). glib_select_fill() will map that to xfds, which we then
[Qemu-devel] [PATCH 32/60] qed: refuse unaligned zero writes with a backing file
From: Stefan Hajnoczi Zero writes have cluster granularity in QED. Therefore they can only be used to zero entire clusters. If the zero write request leaves sectors untouched, zeroing the entire cluster would obscure the backing file. Instead return -ENOTSUP, which is handled by block.c:bdrv_co_do_write_zeroes() and falls back to a regular write. The qemu-iotests 034 test cases covers this scenario. Signed-off-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Signed-off-by: Kevin Wolf (cherry picked from commit ef72f76e58107bd4096018c3db2912d28249308e) Signed-off-by: Michael Tokarev --- block/qed.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/block/qed.c b/block/qed.c index 30a31f9..49ac93e 100644 --- a/block/qed.c +++ b/block/qed.c @@ -1370,10 +1370,21 @@ static int coroutine_fn bdrv_qed_co_write_zeroes(BlockDriverState *bs, int nb_sectors) { BlockDriverAIOCB *blockacb; +BDRVQEDState *s = bs->opaque; QEDWriteZeroesCB cb = { .done = false }; QEMUIOVector qiov; struct iovec iov; +/* Refuse if there are untouched backing file sectors */ +if (bs->backing_hd) { +if (qed_offset_into_cluster(s, sector_num * BDRV_SECTOR_SIZE) != 0) { +return -ENOTSUP; +} +if (qed_offset_into_cluster(s, nb_sectors * BDRV_SECTOR_SIZE) != 0) { +return -ENOTSUP; +} +} + /* Zero writes start without an I/O buffer. If a buffer becomes necessary * then it will be allocated during request processing. */ -- 1.7.10.4
[Qemu-devel] [PATCH 34/60] memory: fix rendering of a region obscured by another
From: Avi Kivity The memory core drops regions that are hidden by another region (for example, during BAR sizing), but it doesn't do so correctly if the lower address of the existing range is below the lower address of the new range. Example (qemu-system-mips -M malta -kernel vmlinux-2.6.32-5-4kc-malta -append "console=ttyS0" -nographic -vga cirrus): Existing range: 1000-107f New range: 100a-100b Correct behaviour: drop new range Incorrect behaviour: add new range Fix by taking this case into account (previously we only considered equal lower boundaries). Tested-by: Aurelien Jarno Signed-off-by: Avi Kivity Signed-off-by: Anthony Liguori (cherry picked from commit d26a8caea3f160782841efb87b5e8bea606b512b) Signed-off-by: Michael Tokarev --- memory.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/memory.c b/memory.c index f039464..d8654fc 100644 --- a/memory.c +++ b/memory.c @@ -538,12 +538,12 @@ static void render_memory_region(FlatView *view, offset_in_region += int128_get64(now); int128_subfrom(&remain, now); } -if (int128_eq(base, view->ranges[i].addr.start)) { -now = int128_min(remain, view->ranges[i].addr.size); -int128_addto(&base, now); -offset_in_region += int128_get64(now); -int128_subfrom(&remain, now); -} +now = int128_sub(int128_min(int128_add(base, remain), +addrrange_end(view->ranges[i].addr)), + base); +int128_addto(&base, now); +offset_in_region += int128_get64(now); +int128_subfrom(&remain, now); } if (int128_nz(remain)) { fr.mr = mr; -- 1.7.10.4
Re: [Qemu-devel] [PATCH 0/4] migration stats fixes
On 02/01/2013 02:32 PM, Juan Quintela wrote: Hi migration expected_downtime calculation was removed on commit e4ed1541ac9413eac494a03532e34beaf8a7d1c5. We add the calculation back. Before doing the calculation we do: - expected_downtime intial value is max_downtime. Much, much better intial value than 0. - we move when we measure the time. We used to measure how much it took "before" we really sent the data. - we introduce sleep_time concept. While we are sleeping because we have sent all the allowed data for this second we shouldn't be accounting that time as "sending". - last patch just introduces the re-calculation of expected_downtime. It just changes the stats value. Well, patchs 2 & 3 change the bandwidth calculation for migration, but I think that we were undercalculating it enough than it was a bug. Without the 2 & 3 patches, the "expected_downtime" for an idle gust was calculated as 80ms (with 30 ms default target value), and we ended having a downtime of around 15ms. With this patches applied, we calculate an expected downtime of around 15ms or so, and then we spent aroqund 18ms on downtime. Notice that we only calculate how much it takes to sent the rest of the RAM, it just happens that there is some more data to sent that what we are calculating. Review, please. Later, Juan. The following changes since commit 8a55ebf01507ab73cc458cfcd5b9cb856aba0b9e: Merge remote-tracking branch 'afaerber/qom-cpu' into staging (2013-01-31 19:37:33 -0600) are available in the git repository at: git://repo.or.cz/qemu/quintela.git stats.next for you to fetch changes up to 791128495e3546ccc88dd037ea4dfd31eca14a56: migration: calculate expected_downtime (2013-02-01 13:22:37 +0100) Juan Quintela (4): migration: change initial value of expected_downtime migration: calculate end time after we have sent the data migration: don't account sleep time for calculating bandwidth migration: calculate expected_downtime arch_init.c | 1 + include/migration/migration.h | 1 + migration.c | 15 +-- 3 files changed, 15 insertions(+), 2 deletions(-) Reviewed-by: Chegu Vinod
Re: [Qemu-devel] Question about default floppy and stdvga memory
Fabio Fantoni writes: > Il 04/02/2013 11:36, Markus Armbruster ha scritto: >> Fabio Fantoni writes: >> >>> I tried to disable "default floppy" without use -nodefaults option >>> that disable other things. >>> I didn't found other parameters to do that in docs and code for now. >>> Can someone tell me if there is another way to disable default floppy only? >> As far as I know, there is none. > > Can someone add an option (for example -nofloppy) for disable default > floppy? Found one: -global isa-fdc.driveA=
[Qemu-devel] [PATCH 02/60] qemu-char: BUGFIX, don't call FD_ISSET with negative fd
From: David Gibson tcp_chr_connect(), unlike for example udp_chr_update_read_handler() does not check if the fd it is using is valid (>= 0) before passing it to qemu_set_fd_handler2(). If using e.g. a TCP serial port, which is not initially connected, this can result in -1 being passed to FD_ISSET, which has undefined behaviour. On x86 it seems to harmlessly return 0, but on PowerPC, it causes a fortify buffer overflow error to be thrown. This patch fixes this by putting an extra test in tcp_chr_connect(), and also adds an assert qemu_set_fd_handler2() to catch other such errors on all platforms, rather than just some. Signed-off-by: David Gibson Signed-off-by: Anthony Liguori (cherry picked from commit bbdd2ad0814ea0911076419ea21b7957505cf1cc) Signed-off-by: Michael Tokarev --- iohandler.c |2 ++ qemu-char.c |6 -- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/iohandler.c b/iohandler.c index dea4355..a2d871b 100644 --- a/iohandler.c +++ b/iohandler.c @@ -56,6 +56,8 @@ int qemu_set_fd_handler2(int fd, { IOHandlerRecord *ioh; +assert(fd >= 0); + if (!fd_read && !fd_write) { QLIST_FOREACH(ioh, &io_handlers, next) { if (ioh->fd == fd) { diff --git a/qemu-char.c b/qemu-char.c index fe1126f..3a68430 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2321,8 +2321,10 @@ static void tcp_chr_connect(void *opaque) TCPCharDriver *s = chr->opaque; s->connected = 1; -qemu_set_fd_handler2(s->fd, tcp_chr_read_poll, - tcp_chr_read, NULL, chr); +if (s->fd >= 0) { +qemu_set_fd_handler2(s->fd, tcp_chr_read_poll, + tcp_chr_read, NULL, chr); +} qemu_chr_generic_open(chr); } -- 1.7.10.4