Re: [Qemu-devel] [PATCH] Give detailed info when pcie downstream port init failed
Cao jin writes: > On 11/27/2015 10:22 PM, Markus Armbruster wrote: >> Cao jin writes: >> >>> Hi, Markus >>> >>> On 11/24/2015 06:08 PM, Markus Armbruster wrote: > [...] >>> >>> and this will cover to output to the monitor, right? >> >> The device still implements the old PCIDeviceClass.init() instead of the >> new .realize(). All devices need to be converted to .realize(). You >> can help by converting this one. >> >> .init() reports errors with error_report() and returns 0 on success, -1 >> on failure. >> >> .realize() passes errors to its callers via its errp argument. >> > > After reading you commit 28b07e7, I am aware of this;) Thanks very > much for your detailed explanation:) I was always curious about why > there are 2 function(.init() & .realize()) for initializing device, > now I guess I get it: it has just .init() at first, because of error > report issue, the .realize() is added to replace it, in order to pass > the error above. Do I understand it right ? Yes! > If I understand it right, I see there are still devices initialization > using .init() of DeviceClass & PCIDeviceClass, maybe I can help to > convert some of these. That would be helpful. There are probably easy cases where you only have to convert one device model at a time, and more complex cases, where you have to convert supporting functions from error_report() to error_set(), or convert device classes, not just individual devices. Best try an easy one first. >> Commit 28b07e7 can serve as example of how to convert a device to >> realize(). There are many more. The ones I did usually have "Convert >> to realize()" in the commit message's first line. >> > >> Regarding six error paths: I counted six places that return -1 directly >> or goto err or similar. Your patch adds a suitable error message to one >> of them. The other five need one, too. You'll have to examine the >> called functions to find out whether they report anything. If they do, >> they need to be first converted to pass an Error to their callers >> instead. >> > > It seems I have a misunderstanding about the "6 error paths":-[ I > thought "error path" is a "error *reporting path*", like: fprintf the > msg to stderr is a path, while output it via QMP is also a > path. Because I did`t touch the code about -chardev/-mon/-monitor/-qmp > before, so I spent 2 days reading the code:) Welcome to the club :) > Now I am cleared, Thanks you very much for the clarifying, Markus, I > think this mistake won`t happen to me later;) And actually, I was > already thinking add suitable error msg to each error path:) Glad I could help you!
[Qemu-devel] [PATCH for 2.5 2/2] pcnet: fix rx buffer overflow(CVE-2015-7512)
Backends could provide a packet whose length is greater than buffer size. Check for this and truncate the packet to avoid rx buffer overflow in this case. Cc: Prasad J Pandit Cc: qemu-sta...@nongnu.org Signed-off-by: Jason Wang --- hw/net/pcnet.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c index 309c40b..1f4a3db 100644 --- a/hw/net/pcnet.c +++ b/hw/net/pcnet.c @@ -1064,6 +1064,12 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_) int pktcount = 0; if (!s->looptest) { +if (size > 4092) { +#ifdef PCNET_DEBUG_RMD +fprintf(stderr, "pcnet: truncates rx packet.\n"); +#endif +size = 4092; +} memcpy(src, buf, size); /* no need to compute the CRC */ src[size] = 0; -- 2.5.0
[Qemu-devel] [PATCH for 2.5 1/2] net: pcnet: add check to validate receive data size(CVE-2015-7504)
From: Prasad J Pandit In loopback mode, pcnet_receive routine appends CRC code to the receive buffer. If the data size given is same as the buffer size, the appended CRC code overwrites 4 bytes after s->buffer. Added a check to avoid that. Reported by: Qinghao Tang Cc: qemu-sta...@nongnu.org Signed-off-by: Prasad J Pandit Signed-off-by: Jason Wang --- hw/net/pcnet.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/hw/net/pcnet.c b/hw/net/pcnet.c index 0eb3cc4..309c40b 100644 --- a/hw/net/pcnet.c +++ b/hw/net/pcnet.c @@ -1084,7 +1084,7 @@ ssize_t pcnet_receive(NetClientState *nc, const uint8_t *buf, size_t size_) uint32_t fcs = ~0; uint8_t *p = src; -while (p != &src[size-4]) +while (p != &src[size]) CRC(fcs, *p++); crc_err = (*(uint32_t *)p != htonl(fcs)); } @@ -1233,8 +1233,10 @@ static void pcnet_transmit(PCNetState *s) bcnt = 4096 - GET_FIELD(tmd.length, TMDL, BCNT); /* if multi-tmd packet outsizes s->buffer then skip it silently. - Note: this is not what real hw does */ -if (s->xmit_pos + bcnt > sizeof(s->buffer)) { + * Note: this is not what real hw does. + * Last four bytes of s->buffer are used to store CRC FCS code. + */ +if (s->xmit_pos + bcnt > sizeof(s->buffer) - 4) { s->xmit_pos = -1; goto txdone; } -- 2.5.0
Re: [Qemu-devel] [PATCH v5 04/10] cpu: Reclaim vCPU objects
On 11/20/2015 11:54 PM, Bharata B Rao wrote: From: Gu Zheng In order to deal well with the kvm vcpus (which can not be removed without any protection), we do not close KVM vcpu fd, just record and mark it as stopped into a list, so that we can reuse it for the appending cpu hot-add request if possible. It is also the approach that kvm guys suggested: https://www.mail-archive.com/kvm@vger.kernel.org/msg102839.html Signed-off-by: Chen Fan Signed-off-by: Gu Zheng Signed-off-by: Zhu Guihua Signed-off-by: Bharata B Rao [- Explicit CPU_REMOVE() from qemu_kvm/tcg_destroy_vcpu() isn't needed as it is done from cpu_exec_exit()] Reviewed-by: David Gibson --- cpus.c | 41 + include/qom/cpu.h| 10 + include/sysemu/kvm.h | 1 + kvm-all.c| 57 +++- kvm-stub.c | 5 + 5 files changed, 113 insertions(+), 1 deletion(-) diff --git a/cpus.c b/cpus.c index 877bd70..af2b274 100644 --- a/cpus.c +++ b/cpus.c @@ -953,6 +953,21 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data) qemu_cpu_kick(cpu); } +static void qemu_kvm_destroy_vcpu(CPUState *cpu) +{ +if (kvm_destroy_vcpu(cpu) < 0) { +error_report("kvm_destroy_vcpu failed.\n"); +exit(EXIT_FAILURE); +} + +object_unparent(OBJECT(cpu)); +} + +static void qemu_tcg_destroy_vcpu(CPUState *cpu) +{ +object_unparent(OBJECT(cpu)); +} + static void flush_queued_work(CPUState *cpu) { struct qemu_work_item *wi; @@ -1053,6 +1068,11 @@ static void *qemu_kvm_cpu_thread_fn(void *arg) } } qemu_kvm_wait_io_event(cpu); +if (cpu->exit && !cpu_can_run(cpu)) { +qemu_kvm_destroy_vcpu(cpu); +qemu_mutex_unlock(&qemu_global_mutex); Nit: qemu_mutex_unlock_iothread() may be? Or it is important for iothread_locked to remain "true"? It does not seem to be used much though. -- Alexey
Re: [Qemu-devel] [RFC PATCH v3 1/5] hw/intc: Implement ITS base class
Hello! > > +/* Our two regions are always adjacent, therefore we now combine them > > + * into a single one in order to make our users' life easier. > > + */ > > +memory_region_init(&s->iomem_main, OBJECT(s), "gicv3_its", ITS_SIZE); > > +memory_region_add_subregion(&s->iomem_main, 0, &s->iomem_its_cntrl); > > +memory_region_add_subregion(&s->iomem_main, ITS_CONTROL_SIZE, > > +&s->iomem_its); > > +sysbus_init_mmio(sbd, &s->iomem_main); > > So we have two memory subregions: > * the control register page, whose ops are passed in by the subclass > * the translation register page, whose only register is implemented >here by looking up the subclass and calling its send_msi method > > Does that complexity gain us anything later? There doesn't really > seem to be anything much actually in common here, which would > suggest just having the subclasses create their own mmio region(s) > (which could then just directly implement the right behaviour for > GITS_TRANSLATER). I started from this, but decided to move region creation into base class, because: 1. We always have this region, both for KVM and for SW implementation, and it operates in the same way. 2. We always need to do address validation, as well as have gicv3_its_trans_read() stub. 3. Also, in the next revision i'll implement 16-bit handling here. So, i decided not to duplicate these things. > > + > > +const char *its_class_name(void) > > +{ > > +if (kvm_irqchip_in_kernel()) { > > +#ifdef TARGET_AARCH64 > > +/* KVM implementation requires this capability */ > > +if (kvm_direct_msi_enabled()) { > > +return "arm-its-kvm"; > > +} > > +#endif > > Why is this in an #ifdef? In theory we could support > the GICv3 in a 32-bit system. Only for code consistency, because "arm-its-kvm" class is compiled only for TARGET_AARCH64, because only there we have the relevant kernel API definitions. So, i did not want to refer to nonexistent class. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia
Re: [Qemu-devel] [RFC PATCH V2 0/3] IXGBE/VFIO: Add live migration support for SRIOV NIC
On 11/26/2015 11:56 AM, Alexander Duyck wrote: > I am not saying you cannot modify the drivers, however what you are doing is far too invasive. Do you seriously plan on modifying all of the PCI device drivers out there in order to allow any device that might be direct assigned to a port to support migration? I certainly hope not. That is why I have said that this solution will not scale. Current drivers are not migration friendly. If the driver wants to support migration, it's necessary to be changed. RFC PATCH V1 presented our ideas about how to deal with MMIO, ring and DMA tracking during migration. These are common for most drivers and they maybe problematic in the previous version but can be corrected later. Doing suspend and resume() may help to do migration easily but some devices requires low service down time. Especially network and I got that some cloud company promised less than 500ms network service downtime. So I think performance effect also should be taken into account when we design the framework. What I am counter proposing seems like a very simple proposition. It can be implemented in two steps. 1. Look at modifying dma_mark_clean(). It is a function called in the sync and unmap paths of the lib/swiotlb.c. If you could somehow modify it to take care of marking the pages you unmap for Rx as being dirty it will get you a good way towards your goal as it will allow you to continue to do DMA while you are migrating the VM. 2. Look at making use of the existing PCI suspend/resume calls that are there to support PCI power management. They have everything needed to allow you to pause and resume DMA for the device before and after the migration while retaining the driver state. If you can implement something that allows you to trigger these calls from the PCI subsystem such as hot-plug then you would have a generic solution that can be easily reproduced for multiple drivers beyond those supported by ixgbevf. Glanced at PCI hotplug code. The hotplug events are triggered by PCI hotplug controller and these event are defined in the controller spec. It's hard to extend more events. Otherwise, we also need to add some specific codes in the PCI hotplug core since it's only add and remove PCI device when it gets events. It's also a challenge to modify Windows hotplug codes. So we may need to find another way. Thanks. - Alex
Re: [Qemu-devel] [RFC PATCH v3 3/5] kvm_arm: Pass requester ID to MSI routing functions
Hello! > > +route->u.msi.devid = pci_requester_id(dev); > > +} > > Is there anything that would go wrong if we just always set > the u.msi.devid and the VALID_DEVID flag? (ie do we need the > kvm_arm_msi_use_devid bool?) Current kernels always make sure that flags == 0, or they simply return -EINVAL. Therefore, for backwards compatibility we set the flag only if we know that we need it. Kind regards, Pavel Fedin Expert Engineer Samsung Electronics Research center Russia
Re: [Qemu-devel] [PATCH for 2.5 1/1] e1000: fix hang of win2k12 shutdown with flood ping
On 11/30/2015 08:58 AM, Jason Wang wrote: On 11/27/2015 07:42 PM, Denis V. Lunev wrote: On 11/27/2015 09:50 AM, Denis V. Lunev wrote: On 11/27/2015 09:48 AM, Denis V. Lunev wrote: e1000 driver in Win2k12 is really well rotten. It 100% hangs on shutdown of UP VM under flood ping. The guest checks card state and reinjects itself interrupt in a loop. This is fatal for UP machine. There is no good way to fix this misbehavior but to kludge it. The emulation has interrupt throttling register aka ITR which limits interrupt rate and allows the guest to proceed this phase. There is no problem with this kludge for Linux guests - it adjust the value of it itself. On the other hand according to the initial research in commit e9845f0985f088dd01790f4821026df0afba5795 Author: Vincenzo Maffione Date: Fri Aug 2 18:30:52 2013 +0200 e1000: add interrupt mitigation support ... Interrupt mitigation boosts performance when the guest suffers from an high interrupt rate (i.e. receiving short UDP packets at high packet rate). For some numerical results see the following link http://info.iet.unipi.it/~luigi/papers/20130520-rizzo-vm.pdf this should also boost performance a bit. See https://bugzilla.redhat.com/show_bug.cgi?id=874406 for additional details. Signed-off-by: Denis V. Lunev CC: Vincenzo Maffione CC: Stefan Hajnoczi --- hw/net/e1000.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/net/e1000.c b/hw/net/e1000.c index c877e06..0af528f 100644 --- a/hw/net/e1000.c +++ b/hw/net/e1000.c @@ -447,6 +447,9 @@ static void e1000_reset(void *opaque) e1000_link_down(d); } +/* Throttle interrupts to allow poor Win 2012 to shutdown */ +d->mac_reg[ITR] = 250; + /* Some guests expect pre-initialized RAH/RAL (AddrValid flag + MACaddr) */ d->mac_reg[RA] = 0; d->mac_reg[RA + 1] = E1000_RAH_AV; Intel manual says about ITR that " A initial suggested range is 651-5580 (28Bh - 15CCh)." Should we use something other than 250? :) http://www.intel.com/content/www/us/en/embedded/products/networking/pci-pci-x-family-gbe-controllers-software-dev-manual.html Den Jason, can you look to this? I have rechecked MAINTAINERs file and found that I have missed you here. Sorry :( Den No problem. But I have a question. What if ITR is disabled? On behalf of guest I do not think that this is really true. In this case the guest should set it to a real value and after that clear it. This is not the case - my patch applies on a reset only, i.e. the guest do not care at all on this and the value lives "as is". I think that real card behaves in a similar way, it could not generate interrupts with the speed of any hypervisor, i.e. there is natural limitation which allows to bypass this problem or there is a default value. On behalf of QEMU the question is still here. Fortunately the handle (mitigation flag) is on by default. I think that it exists to preserve compatibility with QEMU 1.6 In a real life nobody will turn it off until the person is know what he is doing ;) Den
Re: [Qemu-devel] [PATCH v1 1/2] sd: sdhci: Delete over-zealous power check
> -Original Message- > From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com] > Sent: Sunday, November 29, 2015 2:21 AM > To: qemu-devel@nongnu.org; j...@tribudubois.net > Cc: Sai Pavan Boddu; qemu-bl...@nongnu.org; Peter Crosthwaite > Subject: [PATCH v1 1/2] sd: sdhci: Delete over-zealous power check > > This check was conditionalising SD card operation on the card being > powered by the SDHCI host controller. It is however possible > (particularly in embedded systems) for the power control of the SD card > to be managed outside of SDHCI. This can be as trivial as hard-wiring > the SD slot VCC to a constant power-rail. > > This means the guest SDHCI can validly opt-out of the SDHCI power > control feature while still using the card. So delete this check to > allow operation of the card with SDHCI power control. > > This is needed for at least Xilinx Zynq and also makes Freescale i.MX25 > work for me. The digilent Zybo board has a public schematic > which shows SD VCC hardwiring: > > http://digilentinc.com/Data/Products/ZYBO/ZYBO_sch_VB.3.pdf > bottom of page 3. > > Signed-off-by: Peter Crosthwaite Patch is good with the change. Reviewed-by: Sai Pavan Boddu Thanks, Sai Pavan > --- > > hw/sd/sdhci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c > index d70d1a6..ed536ee 100644 > --- a/hw/sd/sdhci.c > +++ b/hw/sd/sdhci.c > @@ -831,7 +831,7 @@ static void sdhci_data_transfer(void *opaque) > > static bool sdhci_can_issue_command(SDHCIState *s) > { > -if (!SDHC_CLOCK_IS_ON(s->clkcon) || !(s->pwrcon & SDHC_POWER_ON) || > +if (!SDHC_CLOCK_IS_ON(s->clkcon) || > (((s->prnsts & SDHC_DATA_INHIBIT) || s->stopped_state) && > ((s->cmdreg & SDHC_CMD_DATA_PRESENT) || > ((s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY && > -- > 1.9.1
Re: [Qemu-devel] [PATCH for 2.5 1/1] e1000: fix hang of win2k12 shutdown with flood ping
On 11/27/2015 07:42 PM, Denis V. Lunev wrote: > On 11/27/2015 09:50 AM, Denis V. Lunev wrote: >> On 11/27/2015 09:48 AM, Denis V. Lunev wrote: >>> e1000 driver in Win2k12 is really well rotten. It 100% hangs on >>> shutdown >>> of UP VM under flood ping. The guest checks card state and reinjects >>> itself interrupt in a loop. This is fatal for UP machine. >>> >>> There is no good way to fix this misbehavior but to kludge it. The >>> emulation has interrupt throttling register aka ITR which limits >>> interrupt rate and allows the guest to proceed this phase. >>> There is no problem with this kludge for Linux guests - it adjust the >>> value of it itself. >>> >>> On the other hand according to the initial research in >>> commit e9845f0985f088dd01790f4821026df0afba5795 >>> Author: Vincenzo Maffione >>> Date: Fri Aug 2 18:30:52 2013 +0200 >>> >>> e1000: add interrupt mitigation support >>> >>> ... >>> >>> Interrupt mitigation boosts performance when the guest suffers >>> from >>> an high interrupt rate (i.e. receiving short UDP packets at >>> high packet >>> rate). For some numerical results see the following link >>> http://info.iet.unipi.it/~luigi/papers/20130520-rizzo-vm.pdf >>> >>> this should also boost performance a bit. >>> >>> See https://bugzilla.redhat.com/show_bug.cgi?id=874406 for additional >>> details. >>> >>> Signed-off-by: Denis V. Lunev >>> CC: Vincenzo Maffione >>> CC: Stefan Hajnoczi >>> --- >>> hw/net/e1000.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/hw/net/e1000.c b/hw/net/e1000.c >>> index c877e06..0af528f 100644 >>> --- a/hw/net/e1000.c >>> +++ b/hw/net/e1000.c >>> @@ -447,6 +447,9 @@ static void e1000_reset(void *opaque) >>> e1000_link_down(d); >>> } >>> +/* Throttle interrupts to allow poor Win 2012 to shutdown */ >>> +d->mac_reg[ITR] = 250; >>> + >>> /* Some guests expect pre-initialized RAH/RAL (AddrValid flag >>> + MACaddr) */ >>> d->mac_reg[RA] = 0; >>> d->mac_reg[RA + 1] = E1000_RAH_AV; >> Intel manual says about ITR that " A initial suggested range is >> 651-5580 (28Bh - 15CCh)." >> Should we use something other than 250? :) >> >> http://www.intel.com/content/www/us/en/embedded/products/networking/pci-pci-x-family-gbe-controllers-software-dev-manual.html >> >> >> Den > > Jason, can you look to this? > > I have rechecked MAINTAINERs file and found that > I have missed you here. Sorry :( > > Den > No problem. But I have a question. What if ITR is disabled?
[Qemu-devel] [PULL 1/3] trace/simple: Fix warning and wrong trace file name for MinGW
On Windows, getpid() always returns an int value, but pid_t (which is expected by the format string) is either a 32 bit or a 64 bit value. Without a type cast (or a modified format string), the compiler prints a warning when building for 64 bit Windows and the resulting trace_file_name will include a wrong pid: trace/simple.c:332:9: warning: format ‘%lld’ expects argument of type ‘long long int’, but argument 2 has type ‘int’ [-Wformat=] Signed-off-by: Stefan Weil --- trace/simple.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/trace/simple.c b/trace/simple.c index 11ad030..56a624c 100644 --- a/trace/simple.c +++ b/trace/simple.c @@ -329,7 +329,8 @@ bool st_set_trace_file(const char *file) g_free(trace_file_name); if (!file) { -trace_file_name = g_strdup_printf(CONFIG_TRACE_FILE, getpid()); +/* Type cast needed for Windows where getpid() returns an int. */ +trace_file_name = g_strdup_printf(CONFIG_TRACE_FILE, (pid_t)getpid()); } else { trace_file_name = g_strdup_printf("%s", file); } -- 2.1.4
[Qemu-devel] [PULL 3/3] w32: Use gcc option -mthreads
QEMU uses threads / coroutines, therefore support for thread local storage and thread safe libraries (-D_MT) must be enabled by using -mthreads. Signed-off-by: Stefan Weil --- configure | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index 979bc55..67801b0 100755 --- a/configure +++ b/configure @@ -727,6 +727,8 @@ if test "$mingw32" = "yes" ; then QEMU_CFLAGS="-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS" # enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later) QEMU_CFLAGS="-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS" + # MinGW needs -mthreads for TLS and macro _MT. + QEMU_CFLAGS="-mthreads $QEMU_CFLAGS" LIBS="-lwinmm -lws2_32 -liphlpapi $LIBS" write_c_skeleton; if compile_prog "" "-liberty" ; then -- 2.1.4
[Qemu-devel] [PULL 2/3] oslib-win32: Change return type of function getpagesize
getpagesize on Linux returns an int. Fix QEMU's implementation for Windows to return an int (instead of size_t), too. This fixes a compiler warning which was introduced recently (commit 093e3c42). Signed-off-by: Stefan Weil --- include/sysemu/os-win32.h | 2 +- util/oslib-win32.c| 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h index 13dcef6..400e098 100644 --- a/include/sysemu/os-win32.h +++ b/include/sysemu/os-win32.h @@ -87,7 +87,7 @@ static inline void os_setup_post(void) {} void os_set_line_buffering(void); static inline void os_set_proc_name(const char *dummy) {} -size_t getpagesize(void); +int getpagesize(void); #if !defined(EPROTONOSUPPORT) # define EPROTONOSUPPORT EINVAL diff --git a/util/oslib-win32.c b/util/oslib-win32.c index 09f9e98..6a47019 100644 --- a/util/oslib-win32.c +++ b/util/oslib-win32.c @@ -454,7 +454,7 @@ gint g_poll(GPollFD *fds, guint nfds, gint timeout) return retval; } -size_t getpagesize(void) +int getpagesize(void) { SYSTEM_INFO system_info; -- 2.1.4
[Qemu-devel] [PULL 0/3] wxx: Last minute fixes for 2.5
The following changes since commit 714487515dbe0c65d5904251e796cd3a5b3579fb: Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging (2015-11-27 10:44:42 +) are available in the git repository at: git://qemu.weilnetz.de/qemu.git tags/pull-wxx-20151130 for you to fetch changes up to 78e9d4ad11e7116376328860a58b96765ade7b62: w32: Use gcc option -mthreads (2015-11-30 06:47:02 +0100) wxx patch queue Stefan Weil (3): trace/simple: Fix warning and wrong trace file name for MinGW oslib-win32: Change return type of function getpagesize w32: Use gcc option -mthreads configure | 2 ++ include/sysemu/os-win32.h | 2 +- trace/simple.c| 3 ++- util/oslib-win32.c| 2 +- 4 files changed, 6 insertions(+), 3 deletions(-)
Re: [Qemu-devel] [RFC PATCH 3/9] colo-proxy: add colo-proxy framework
On 11/30/2015 11:10 AM, Wen Congyang wrote: On 11/27/2015 08:27 PM, Zhang Chen wrote: From: zhangchen Colo-proxy is a plugin of qemu netfilter like filter-buffer and dump Signed-off-by: zhangchen --- net/Makefile.objs | 1 + net/colo-proxy.c | 139 ++ net/colo-proxy.h | 63 + 3 files changed, 203 insertions(+) create mode 100644 net/colo-proxy.c create mode 100644 net/colo-proxy.h diff --git a/net/Makefile.objs b/net/Makefile.objs index 5fa2f97..95670f2 100644 --- a/net/Makefile.objs +++ b/net/Makefile.objs @@ -15,3 +15,4 @@ common-obj-$(CONFIG_VDE) += vde.o common-obj-$(CONFIG_NETMAP) += netmap.o common-obj-y += filter.o common-obj-y += filter-buffer.o +common-obj-y += colo-proxy.o diff --git a/net/colo-proxy.c b/net/colo-proxy.c new file mode 100644 index 000..98c2699 --- /dev/null +++ b/net/colo-proxy.c @@ -0,0 +1,139 @@ +/* + * COarse-grain LOck-stepping Virtual Machines for Non-stop Service (COLO) + * (a.k.a. Fault Tolerance or Continuous Replication) + * + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO., LTD. + * Copyright (c) 2015 FUJITSU LIMITED + * Copyright (c) 2015 Intel Corporation + * + * Author: Zhang Chen + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * later. See the COPYING file in the top-level directory. + */ + +#include "colo-proxy.h" + +#define __DEBUG__ + +#ifdef __DEBUG__ +#define DEBUG(format, ...) printf(format, ##__VA_ARGS__) +#else +#define DEBUG(format, ...) +#endif + + +static ssize_t colo_proxy_receive_iov(NetFilterState *nf, + NetClientState *sender, + unsigned flags, + const struct iovec *iov, + int iovcnt, + NetPacketSent *sent_cb) +{ +/* + * We return size when buffer a packet, the sender will take it as + * a already sent packet, so sent_cb should not be called later. + * + */ +ColoProxyState *s = FILTER_COLO_PROXY(nf); +if (s->colo_mode == COLO_PRIMARY_MODE) { + /* colo_proxy_primary_handler */ +} else { + /* colo_proxy_primary_handler */ +} +return iov_size(iov, iovcnt); +} + +static void colo_proxy_cleanup(NetFilterState *nf) +{ + /* cleanup */ +} + + +static void colo_proxy_setup(NetFilterState *nf, Error **errp) +{ +ColoProxyState *s = FILTER_COLO_PROXY(nf); +if (!s->addr) { +error_setg(errp, "filter colo_proxy needs 'addr' \ + property set!"); +return; +} + +if (nf->direction != NET_FILTER_DIRECTION_ALL) { +printf("colo need queue all packet,\ s/need/needs/ fix thanks for review zhangchen +please startup colo-proxy with queue=all\n"); +return; +} + +s->sockfd = -1; +s->has_failover = false; +colo_do_checkpoint = false; +g_queue_init(&s->unprocessed_connections); + +if (!strcmp(mode, PRIMARY_MODE)) { +s->colo_mode = COLO_PRIMARY_MODE; +} else if (!strcmp(mode, SECONDARY_MODE)) { +s->colo_mode = COLO_SECONDARY_MODE; +} else { +error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "mode", +"primary or secondary"); +return; +} +} + +static void colo_proxy_class_init(ObjectClass *oc, void *data) +{ +NetFilterClass *nfc = NETFILTER_CLASS(oc); + +nfc->setup = colo_proxy_setup; +nfc->cleanup = colo_proxy_cleanup; +nfc->receive_iov = colo_proxy_receive_iov; +} + +static char *colo_proxy_get_mode(Object *obj, Error **errp) +{ +return g_strdup(mode); +} + +static void colo_proxy_set_mode(Object *obj, const char *value, Error **errp) +{ +g_free(mode); +mode = g_strdup(value); +} + +static char *colo_proxy_get_addr(Object *obj, Error **errp) +{ +ColoProxyState *s = FILTER_COLO_PROXY(obj); + +return g_strdup(s->addr); +} + +static void colo_proxy_set_addr(Object *obj, const char *value, Error **errp) +{ +ColoProxyState *s = FILTER_COLO_PROXY(obj); +g_free(s->addr); +s->addr = g_strdup(value); You can parse the address here, and can find the format error as early as possible. ok,it will fix in next version +} + +static void colo_proxy_init(Object *obj) +{ +object_property_add_str(obj, "mode", colo_proxy_get_mode, +colo_proxy_set_mode, NULL); +object_property_add_str(obj, "addr", colo_proxy_get_addr, +colo_proxy_set_addr, NULL); +} + +static const TypeInfo colo_proxy_info = { +.name = TYPE_FILTER_COLO_PROXY, +.parent = TYPE_NETFILTER, +.class_init = colo_proxy_class_init, +.instance_init = colo_proxy_init, +.instance_size = sizeof(ColoProxyState), +}; + +static void register_types(void) +{ +type_register_static(&colo_proxy_info); +} + +type_init(register_types); diff --git a/net/
Re: [Qemu-devel] [PATCH 1/4] vmxnet3: The vmxnet3 device is a PCIE endpoint
On 11/30/2015 05:07 AM, Shmulik Ladkani wrote: > Hi, > > On Wed, 25 Nov 2015 16:24:39 +0800 Jason Wang wrote: > @@ -2568,6 +2572,7 @@ static void vmxnet3_class_init(ObjectClass *class, > void *data) > c->class_id = PCI_CLASS_NETWORK_ETHERNET; > c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE; > c->subsystem_id = PCI_DEVICE_ID_VMWARE_VMXNET3; > +c->is_express = 1; Should we do this conditionally? And how about the migration compatibility? Looks like pcie device is using vmstate_pcie_device instead of vmstate_pci_device, maybe need a new property bit for this. >>> (Responding for the entire series) >>> >>> Agreed. Will limit these changes for new versions. >>> >>> What's your suggested plan? >>> Does it make sense to have a property for each change (as they are not >>> necessarily related), or is it too tedious and one property will suffice? >> Since they are not necessarily related, we'd better use a property for >> each change. > Would it make sense if we expose a new vmxnet3 type to differenciate > pcie vs pci instances of vmxnet3? > > Otherwise, migration gets more complicated, as we need to use either > vmstate_pci_device or vmstate_pcie_device; also, upon vm load, we need > to preserve the semantics saved (whether the instance was pci or pcie). > > I have managed to do so, but is a bit tedious; Exposing a new type seems > cleaner. Yes, it's a good idea to have a new type. Thanks
[Qemu-devel] Question about nonblocking stderr and lost logs
Hi QEMU programmers, While doing some experimental work on QEMU that has involved adding a lot of new log messages (using qemu_log_mask()), I've discovered that under some conditions a lot of my log messages go missing. I've tracked the issue down to qemu_logfile being left at the default (stderr) (so when not using -D) and according to strace what is happening is that the debug messages are being passed to write() but write() is returning EWOULDBLOCK and the messags are dropped. This seems to be happening because stderr is being set non-blocking (which is a bit odd to me), and it seems like NONBLOCK is being set as qmp_chardev_add() adds a device for stdout (yes stdout, not stderr; perhaps file descriptors have been dup'd by that point?). Is this by design to prevent a slow console from blocking QEMU? If not, should I delve further and try to prevent non-blocking being set on stderr? (Unfortunately I don't have a replication for this using an unmodified QEMU but I suspect I could find one if necessary.) Cheers, Sam.
Re: [Qemu-devel] [RFC PATCH 1/9] Init colo-proxy object based on netfilter
On 11/30/2015 10:50 AM, Wen Congyang wrote: On 11/27/2015 08:27 PM, Zhang Chen wrote: From: zhangchen add colo-proxy in vl.c and qemu-options.hx Signed-off-by: zhangchen --- qemu-options.hx | 4 vl.c| 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index 949db7f..5e6f1e3 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -3666,6 +3666,10 @@ queue @var{all|rx|tx} is an option that can be applied to any netfilter. @option{tx}: the filter is attached to the transmit queue of the netdev, where it will receive packets sent by the netdev. +@item -object colo-proxy,id=@var{id},netdev=@var{netdevid},port=@var{t},addr=@var{ip:port},mode=@var{primary|secondary}[,queue=@var{all|rx|tx}] 1. queue *MUST* be all for the filter colo-proxy. 2. The option port should be removed 3. The option addr is socket address. The format can be host:port, or fd. will fix in next version thanks for review zhangchen + +colo-proxy Add more description here. Thanks Wen Congyang will fix in next version + @item -object filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}] Dump the network traffic on netdev @var{dev} to the file specified by diff --git a/vl.c b/vl.c index f5f7c3f..9037743 100644 --- a/vl.c +++ b/vl.c @@ -2774,7 +2774,8 @@ static bool object_create_initial(const char *type) * they depend on netdevs already existing */ if (g_str_equal(type, "filter-buffer") || -g_str_equal(type, "filter-dump")) { +g_str_equal(type, "filter-dump") || +g_str_equal(type, "colo-proxy")) { return false; } .
Re: [Qemu-devel] [RFC PATCH 5/9] net/colo-proxy: add colo packet handler
On 11/28/2015 11:17 AM, Hailiang Zhang wrote: On 2015/11/27 20:27, Zhang Chen wrote: From: zhangchen add primary and secondary handler Signed-off-by: zhangchen --- net/colo-proxy.c | 105 +-- 1 file changed, 103 insertions(+), 2 deletions(-) diff --git a/net/colo-proxy.c b/net/colo-proxy.c index 89d9616..ece5661 100644 --- a/net/colo-proxy.c +++ b/net/colo-proxy.c @@ -25,6 +25,101 @@ static char *mode; static bool colo_do_checkpoint; +/* + * colo primary handle host's normal send and + * recv packets to primary guest + * return: >= 0 success + * < 0 failed + */ +static ssize_t colo_proxy_primary_handler(NetFilterState *nf, + NetClientState *sender, + unsigned flags, + const struct iovec *iov, + int iovcnt, + NetPacketSent *sent_cb) +{ +ssize_t ret = 0; +int direction; + +if (sender == nf->netdev) { +/* This packet is sent by netdev itself */ +direction = NET_FILTER_DIRECTION_TX; +} else { +direction = NET_FILTER_DIRECTION_RX; +} +/* + * if packet's direction=rx + * enqueue packets to primary queue + * and wait secondary queue to compare + * if packet's direction=tx + * enqueue packets then send packets to + * secondary and flush queued packets +*/ + +if (colo_do_checkpoint) { +colo_proxy_do_checkpoint(nf); +} + Wrong patch ? Where is the definition of colo_proxy_do_checkpoint() ? sorry,the definition in patch 9/9,in next version I will replace it with /* colo_proxy_do_checkpoint */ thanks for review zhangchen Besides, why did we need to call colo_proxy_do_checkpoint() here ? if proxy compare modles find packet different,it will nofity colo to do checkpoint (use colo_proxy_notify_checkpoint).then proxy wait colo to respond and change colo_do_checkpoint = true,in that time proxy flush queued primary packet. the location we call colo_proxy_do_checkpoint() will fix in next version. +if (direction == NET_FILTER_DIRECTION_RX) { +/* TODO: enqueue_primary_packet */ +} else { +/* TODO: forward packets to another */ +} + +return ret; +} + +/* + * colo secondary handle host's normal send and + * recv packets to secondary guest + * return: >= 0 success + * < 0 failed + */ +static ssize_t colo_proxy_secondary_handler(NetFilterState *nf, + NetClientState *sender, + unsigned flags, + const struct iovec *iov, + int iovcnt, + NetPacketSent *sent_cb) +{ +ColoProxyState *s = FILTER_COLO_PROXY(nf); +int direction; +ssize_t ret = 0; + +if (sender == nf->netdev) { +/* This packet is sent by netdev itself */ +direction = NET_FILTER_DIRECTION_TX; +} else { +direction = NET_FILTER_DIRECTION_RX; +} +/* + * if packet's direction=rx + * enqueue packets and send to + * primary QEMU + * if packet's direction=tx + * record PVM's packet inital seq & adjust + * client's ack,send adjusted packets to SVM(next version will be do) + */ + +if (direction == NET_FILTER_DIRECTION_RX) { +if (colo_has_failover(nf)) { +qemu_net_queue_send_iov(s->incoming_queue, sender, flags, iov, +iovcnt, NULL); +return 1; +} else { +/* TODO: forward packets to another */ +} + +} else { +if (colo_has_failover(nf)) { +qemu_net_queue_send_iov(s->incoming_queue, sender, flags, iov, +iovcnt, NULL); +} +return 1; These codes can be placed outside of the outer if/else. fix +} +return ret; +} + static ssize_t colo_proxy_receive_iov(NetFilterState *nf, NetClientState *sender, unsigned flags, @@ -38,10 +133,16 @@ static ssize_t colo_proxy_receive_iov(NetFilterState *nf, * */ ColoProxyState *s = FILTER_COLO_PROXY(nf); +ssize_t ret = 0; Space ~ fix if (s->colo_mode == COLO_PRIMARY_MODE) { - /* colo_proxy_primary_handler */ +ret = colo_proxy_primary_handler(nf, sender, flags, +iov, iovcnt, sent_cb); } else { - /* colo_proxy_primary_handler */ +ret = colo_proxy_secondary_handler(nf, sender, flags, +iov, iovcnt, sent_cb); +} +if (ret < 0) { +DEBUG("colo_proxy_receive_iov running failed\n"); } return iov_size(iov, io
Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35
On 11/29/15 13:37, Marcel Apfelbaum wrote: > On 11/26/2015 07:01 PM, Laszlo Ersek wrote: >> Hello Marcel, >> > > [...] if you have ACPI table dumps from within an i440fx >> SeaBIOS Linux guest, both from before and after your QEMU patches, and >> those dumps are identical, then that's good evidence against >> regressions. (I tend to do such acpidump-based comparisons when messing >> with ACPI builder code.) >> > > Hi, > > OK, there are no functional differences between the SSDT before/after, > however the optimization made in patch "1/3 hw/acpi: merge pxb adjacent > memory/IO ranges" > for pxb-pcies works also for pxb, which is a good thing. > > SSDT before (only PXB differences) : > --- > > Device (PC0A) > { >... > Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings > { > ... > DWordMemory (ResourceProducer, PosDecode, MinFixed, > MaxFixed, NonCacheable, ReadWrite, > 0x, // Granularity > 0xFE80, // Range Minimum > 0xFE9F, // Range Maximum > 0x, // Translation Offset > 0x0020, // Length > ,, , AddressRangeMemory, TypeStatic) > DWordMemory (ResourceProducer, PosDecode, MinFixed, > MaxFixed, NonCacheable, ReadWrite, > 0x, // Granularity > 0xFE00, // Range Minimum > 0xFE7F, // Range Maximum > 0x, // Translation Offset > 0x0080, // Length > ,, , AddressRangeMemory, TypeStatic) > ... > }) > } > > SSDT after: > > > Device (PC0A) > { > ... > Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings > { > ... > DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, > NonCacheable, ReadWrite, > 0x, // Granularity > 0xFE00, // Range Minimum > 0xFE9F, // Range Maximum > 0x, // Translation Offset > 0x00A0, // Length > ,, , AddressRangeMemory, TypeStatic) >... > }) > } > > As it can be seen, the optimization works also for PXB by merging the > MEM regions. > > Thanks, > Marcel > > [...] Looks good and makes sense, thanks. Laszlo
[Qemu-devel] question: about exec/poison.h
Hi, all, I met one problem when trying to add a new public function in dump.h named "dump_state_get_global" and using it in hmp.c. What I got is something like: In file included from /root/git/qemu/hmp.c:35:0: /root/git/qemu/include/sysemu/dump.h:26:34: error: attempt to use poisoned "TARGET_PAGE_BITS" (((unsigned long long)(X) >> TARGET_PAGE_BITS) - ARCH_PFN_OFFSET) I did a quick look on the poison.h file, seeing that it should be used to avoid using arch-depentent macros in arch-independent codes. That's cool. However, that's also problem to me. The problem is: First of all, dump itself is arch dependent. Meanwhile, hmp.c is not. Also, what I am trying to add is arch indepentent too (just like hmp.c). Now if I include "dump.h" in hmp.c to use that function, I may encounter the error message. I got one idea, which is to split dump.h into two header files: dump.h and dump-arch-indep.h (the latter name could be of course shorter). So that I can move arch independent declarations into that new header file and use it in hmp.h. Not sure whether this is the good one to go. Does anyone have suggestion on what I should do? Thanks in advance! Peter
Re: [Qemu-devel] [PATCH 07/40] virtio: slim down allocation of VirtQueueElements
On Tue, 11/24 19:00, Paolo Bonzini wrote: > Build the addresses and s/g lists on the stack, and then copy them > to a VirtQueueElement that is just as big as required to contain this > particular s/g list. The cost of the copy is minimal compared to that > of a large malloc. > > When virtqueue_map is used on the destination side of migration or on > loadvm, the iovecs have already been split at memory region boundary, > so we can just reuse the out_num/in_num we find in the file. > > Signed-off-by: Paolo Bonzini > --- > hw/virtio/virtio.c | 82 > +- > 1 file changed, 51 insertions(+), 31 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 32c89eb..0163d0f 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -448,6 +448,32 @@ int virtqueue_avail_bytes(VirtQueue *vq, unsigned int > in_bytes, > return in_bytes <= in_total && out_bytes <= out_total; > } > > +static void virtqueue_map_desc(unsigned int *p_num_sg, hwaddr *addr, struct > iovec *iov, > + unsigned int max_num_sg, bool is_write, > + hwaddr pa, size_t sz) > +{ > +unsigned num_sg = *p_num_sg; > +assert(num_sg <= max_num_sg); > + > +while (sz) { > +hwaddr len = sz; > + > +if (num_sg == max_num_sg) { > +error_report("virtio: too many write descriptors in indirect > table"); > +exit(1); > +} > + > +iov[num_sg].iov_base = cpu_physical_memory_map(pa, &len, is_write); > +iov[num_sg].iov_len = len; > +addr[num_sg] = pa; > + > +sz -= len; > +pa += len; > +num_sg++; > +} > +*p_num_sg = num_sg; > +} > + > static void virtqueue_map_iovec(struct iovec *sg, hwaddr *addr, > unsigned int *num_sg, unsigned int max_size, > int is_write) > @@ -474,20 +500,10 @@ static void virtqueue_map_iovec(struct iovec *sg, > hwaddr *addr, > error_report("virtio: error trying to map MMIO memory"); > exit(1); > } > -if (len == sg[i].iov_len) { > -continue; > -} > -if (*num_sg >= max_size) { > -error_report("virtio: memory split makes iovec too large"); > +if (len != sg[i].iov_len) { > +error_report("virtio: unexpected memory split"); > exit(1); > } > -memmove(sg + i + 1, sg + i, sizeof(*sg) * (*num_sg - i)); > -memmove(addr + i + 1, addr + i, sizeof(*addr) * (*num_sg - i)); > -assert(len < sg[i + 1].iov_len); > -sg[i].iov_len = len; > -addr[i + 1] += len; > -sg[i + 1].iov_len -= len; > -++*num_sg; > } > } > > @@ -525,14 +541,16 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) > hwaddr desc_pa = vq->vring.desc; > VirtIODevice *vdev = vq->vdev; > VirtQueueElement *elem; > +unsigned out_num, in_num; > +hwaddr addr[VIRTQUEUE_MAX_SIZE]; > +struct iovec iov[VIRTQUEUE_MAX_SIZE]; > > if (!virtqueue_num_heads(vq, vq->last_avail_idx)) { > return NULL; > } > > /* When we start there are none of either input nor output. */ > -elem = virtqueue_alloc_element(sz, VIRTQUEUE_MAX_SIZE, > VIRTQUEUE_MAX_SIZE); > -elem->out_num = elem->in_num = 0; > +out_num = in_num = 0; > > max = vq->vring.num; > > @@ -555,37 +573,39 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz) > > /* Collect all the descriptors */ > do { > -struct iovec *sg; > +hwaddr pa = vring_desc_addr(vdev, desc_pa, i); > +size_t len = vring_desc_len(vdev, desc_pa, i); > > if (vring_desc_flags(vdev, desc_pa, i) & VRING_DESC_F_WRITE) { > -if (elem->in_num >= VIRTQUEUE_MAX_SIZE) { > -error_report("Too many write descriptors in indirect table"); > -exit(1); > -} > -elem->in_addr[elem->in_num] = vring_desc_addr(vdev, desc_pa, i); > -sg = &elem->in_sg[elem->in_num++]; > +virtqueue_map_desc(&in_num, addr + out_num, iov + out_num, > + VIRTQUEUE_MAX_SIZE - out_num, 1, pa, len); > } else { > -if (elem->out_num >= VIRTQUEUE_MAX_SIZE) { > -error_report("Too many read descriptors in indirect table"); > +if (in_num) { > +error_report("Incorrect order for descriptors"); > exit(1); > } > -elem->out_addr[elem->out_num] = vring_desc_addr(vdev, desc_pa, > i); > -sg = &elem->out_sg[elem->out_num++]; > +virtqueue_map_desc(&out_num, addr, iov, > + VIRTQUEUE_MAX_SIZE, 0, pa, len); > } > > -sg->iov_len = vring_desc_len(vdev, desc_pa, i); > - > /* If we've got too many, that implies a descriptor loop. */ > -
Re: [Qemu-devel] [RFC PATCH 3/9] colo-proxy: add colo-proxy framework
On 11/27/2015 08:27 PM, Zhang Chen wrote: > From: zhangchen > > Colo-proxy is a plugin of qemu netfilter > like filter-buffer and dump > > Signed-off-by: zhangchen > --- > net/Makefile.objs | 1 + > net/colo-proxy.c | 139 > ++ > net/colo-proxy.h | 63 + > 3 files changed, 203 insertions(+) > create mode 100644 net/colo-proxy.c > create mode 100644 net/colo-proxy.h > > diff --git a/net/Makefile.objs b/net/Makefile.objs > index 5fa2f97..95670f2 100644 > --- a/net/Makefile.objs > +++ b/net/Makefile.objs > @@ -15,3 +15,4 @@ common-obj-$(CONFIG_VDE) += vde.o > common-obj-$(CONFIG_NETMAP) += netmap.o > common-obj-y += filter.o > common-obj-y += filter-buffer.o > +common-obj-y += colo-proxy.o > diff --git a/net/colo-proxy.c b/net/colo-proxy.c > new file mode 100644 > index 000..98c2699 > --- /dev/null > +++ b/net/colo-proxy.c > @@ -0,0 +1,139 @@ > +/* > + * COarse-grain LOck-stepping Virtual Machines for Non-stop Service (COLO) > + * (a.k.a. Fault Tolerance or Continuous Replication) > + * > + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO., LTD. > + * Copyright (c) 2015 FUJITSU LIMITED > + * Copyright (c) 2015 Intel Corporation > + * > + * Author: Zhang Chen > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or > + * later. See the COPYING file in the top-level directory. > + */ > + > +#include "colo-proxy.h" > + > +#define __DEBUG__ > + > +#ifdef __DEBUG__ > +#define DEBUG(format, ...) printf(format, ##__VA_ARGS__) > +#else > +#define DEBUG(format, ...) > +#endif > + > + > +static ssize_t colo_proxy_receive_iov(NetFilterState *nf, > + NetClientState *sender, > + unsigned flags, > + const struct iovec *iov, > + int iovcnt, > + NetPacketSent *sent_cb) > +{ > +/* > + * We return size when buffer a packet, the sender will take it as > + * a already sent packet, so sent_cb should not be called later. > + * > + */ > +ColoProxyState *s = FILTER_COLO_PROXY(nf); > +if (s->colo_mode == COLO_PRIMARY_MODE) { > + /* colo_proxy_primary_handler */ > +} else { > + /* colo_proxy_primary_handler */ > +} > +return iov_size(iov, iovcnt); > +} > + > +static void colo_proxy_cleanup(NetFilterState *nf) > +{ > + /* cleanup */ > +} > + > + > +static void colo_proxy_setup(NetFilterState *nf, Error **errp) > +{ > +ColoProxyState *s = FILTER_COLO_PROXY(nf); > +if (!s->addr) { > +error_setg(errp, "filter colo_proxy needs 'addr' \ > + property set!"); > +return; > +} > + > +if (nf->direction != NET_FILTER_DIRECTION_ALL) { > +printf("colo need queue all packet,\ s/need/needs/ > +please startup colo-proxy with queue=all\n"); > +return; > +} > + > +s->sockfd = -1; > +s->has_failover = false; > +colo_do_checkpoint = false; > +g_queue_init(&s->unprocessed_connections); > + > +if (!strcmp(mode, PRIMARY_MODE)) { > +s->colo_mode = COLO_PRIMARY_MODE; > +} else if (!strcmp(mode, SECONDARY_MODE)) { > +s->colo_mode = COLO_SECONDARY_MODE; > +} else { > +error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "mode", > +"primary or secondary"); > +return; > +} > +} > + > +static void colo_proxy_class_init(ObjectClass *oc, void *data) > +{ > +NetFilterClass *nfc = NETFILTER_CLASS(oc); > + > +nfc->setup = colo_proxy_setup; > +nfc->cleanup = colo_proxy_cleanup; > +nfc->receive_iov = colo_proxy_receive_iov; > +} > + > +static char *colo_proxy_get_mode(Object *obj, Error **errp) > +{ > +return g_strdup(mode); > +} > + > +static void colo_proxy_set_mode(Object *obj, const char *value, Error **errp) > +{ > +g_free(mode); > +mode = g_strdup(value); > +} > + > +static char *colo_proxy_get_addr(Object *obj, Error **errp) > +{ > +ColoProxyState *s = FILTER_COLO_PROXY(obj); > + > +return g_strdup(s->addr); > +} > + > +static void colo_proxy_set_addr(Object *obj, const char *value, Error **errp) > +{ > +ColoProxyState *s = FILTER_COLO_PROXY(obj); > +g_free(s->addr); > +s->addr = g_strdup(value); You can parse the address here, and can find the format error as early as possible. > +} > + > +static void colo_proxy_init(Object *obj) > +{ > +object_property_add_str(obj, "mode", colo_proxy_get_mode, > +colo_proxy_set_mode, NULL); > +object_property_add_str(obj, "addr", colo_proxy_get_addr, > +colo_proxy_set_addr, NULL); > +} > + > +static const TypeInfo colo_proxy_info = { > +.name = TYPE_FILTER_COLO_PROXY, > +.parent = TYPE_NETFILTER, > +.class_init = colo_proxy_class_init, > +.instance_init = colo_pro
Re: [Qemu-devel] [PATCH 03/40] virtio: move allocation to virtqueue_pop/vring_pop
On Tue, 11/24 19:00, Paolo Bonzini wrote: > @@ -436,10 +454,11 @@ static void control_out(VirtIODevice *vdev, VirtQueue > *vq) > buf = g_malloc(cur_len); > len = cur_len; > } > -iov_to_buf(elem.out_sg, elem.out_num, 0, buf, cur_len); > +iov_to_buf(elem->out_sg, elem->out_num, 0, buf, cur_len); > > handle_control_message(vser, buf, cur_len); > -virtqueue_push(vq, &elem, 0); > +virtqueue_push(vq, elem, 0); > + g_free(elem); s/^\t// ?
Re: [Qemu-devel] [RFC PATCH 1/9] Init colo-proxy object based on netfilter
On 11/27/2015 08:27 PM, Zhang Chen wrote: > From: zhangchen > > add colo-proxy in vl.c and qemu-options.hx > > Signed-off-by: zhangchen > --- > qemu-options.hx | 4 > vl.c| 3 ++- > 2 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/qemu-options.hx b/qemu-options.hx > index 949db7f..5e6f1e3 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -3666,6 +3666,10 @@ queue @var{all|rx|tx} is an option that can be applied > to any netfilter. > @option{tx}: the filter is attached to the transmit queue of the netdev, > where it will receive packets sent by the netdev. > > +@item -object > colo-proxy,id=@var{id},netdev=@var{netdevid},port=@var{t},addr=@var{ip:port},mode=@var{primary|secondary}[,queue=@var{all|rx|tx}] 1. queue *MUST* be all for the filter colo-proxy. 2. The option port should be removed 3. The option addr is socket address. The format can be host:port, or fd. > + > +colo-proxy Add more description here. Thanks Wen Congyang > + > @item -object > filter-dump,id=@var{id},netdev=@var{dev},file=@var{filename}][,maxlen=@var{len}] > > Dump the network traffic on netdev @var{dev} to the file specified by > diff --git a/vl.c b/vl.c > index f5f7c3f..9037743 100644 > --- a/vl.c > +++ b/vl.c > @@ -2774,7 +2774,8 @@ static bool object_create_initial(const char *type) > * they depend on netdevs already existing > */ > if (g_str_equal(type, "filter-buffer") || > -g_str_equal(type, "filter-dump")) { > +g_str_equal(type, "filter-dump") || > +g_str_equal(type, "colo-proxy")) { > return false; > } > >
Re: [Qemu-devel] [Qemu-ppc] [PATCH for-2.5?] hw/ppc/ppc405_boards: Fix infinite recursion by converting taihu_cpld from old_mmio
On Tue, Nov 17, 2015 at 10:43:36AM +0100, Paolo Bonzini wrote: > > > On 16/11/2015 15:57, Peter Maydell wrote: > > The taihu_cpld_writel() function had an obvious typo that meant that > > if it was ever called it would go into an infinite recursion. Newer > > versions of clang will detect and warn about this: > > hw/ppc/ppc405_boards.c:481:1: warning: all paths through this function > > will call itself [-Winfinite-recursion] > > > > Fix this by converting taihu_cpld from the legacy old_mmio accessors > > to new-style ones, with an impl {} declaration to cause the core > > memory code to do the splitting of 16 bit and 32 bit accesses into > > multiple 8-bit accesses. > > > > Signed-off-by: Peter Maydell > > --- > > Marked 'for-2.5?' because of the infinite recursion (though the bug > > has been present since the board support was first committed in 2007). > > NB that I don't have a Taihu image that would exercise the device. > > There would obviously be a smaller fix that just dealt with the recursion > > problem, but old_mmio is an obsolete interface we should be switching > > away from anyhow. > > Yes, it makes sense and the diffstat is nice too. :) > > Reviewed-by: Paolo Bonzini Applied to my for-2.5 tree. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [RFC PATCH 4/9] colo-proxy: add colo-proxy setup work
On 11/28/2015 11:02 AM, Hailiang Zhang wrote: On 2015/11/27 20:27, Zhang Chen wrote: From: zhangchen Secondary setup socket server for colo-forward primary setup connect to secondary for colo-forward add data structure will be uesed Signed-off-by: zhangchen --- net/colo-proxy.c | 87 +++- net/colo-proxy.h | 61 +++ 2 files changed, 147 insertions(+), 1 deletion(-) diff --git a/net/colo-proxy.c b/net/colo-proxy.c index 98c2699..89d9616 100644 --- a/net/colo-proxy.c +++ b/net/colo-proxy.c @@ -22,6 +22,8 @@ #define DEBUG(format, ...) #endif +static char *mode; +static bool colo_do_checkpoint; static ssize_t colo_proxy_receive_iov(NetFilterState *nf, NetClientState *sender, @@ -46,13 +48,84 @@ static ssize_t colo_proxy_receive_iov(NetFilterState *nf, static void colo_proxy_cleanup(NetFilterState *nf) { - /* cleanup */ +ColoProxyState *s = FILTER_COLO_PROXY(nf); +close(s->sockfd); +s->sockfd = -1; +g_free(mode); +g_free(s->addr); } Please move the above codes to the previous patch~ ok thanks for review zhangchen +static void colo_accept_incoming(ColoProxyState *s) +{ +DEBUG("into colo_accept_incoming\n"); +struct sockaddr_in addr; +socklen_t addrlen = sizeof(addr); +int acceptsock, err; + +do { +acceptsock = qemu_accept(s->sockfd, (struct sockaddr *)&addr, &addrlen); +err = socket_error(); +} while (acceptsock < 0 && err == EINTR); +qemu_set_fd_handler(s->sockfd, NULL, NULL, NULL); +closesocket(s->sockfd); + +DEBUG("accept colo proxy\n"); + It's better to use trace instead of DEBUG~ ok,i will fix it in next version +if (acceptsock < 0) { +printf("could not accept colo connection (%s)\n", + strerror(err)); /printf/error_report/g fix +return; +} +s->sockfd = acceptsock; +/* TODO: handle the packets that primary forward */ +return; +} + +/* Return 1 on success, or return -1 if failed */ +static ssize_t colo_start_incoming(ColoProxyState *s) +{ +int serversock; Space~ fix +serversock = inet_listen(s->addr, NULL, 256, SOCK_STREAM, 0, NULL); +if (serversock < 0) { +g_free(s->addr); +return -1; +} +s->sockfd = serversock; +qemu_set_fd_handler(serversock, (IOHandler *)colo_accept_incoming, NULL, +(void *)s); +g_free(s->addr); Double free ? I noticed you also free this in colo_proxy_cleanup(), we'd better do this in the cleanup function. fix,thanks +return 1; Odd, it's better to return 0 to indicate success. will fix in next version +} + +/* Return 1 on success, or return -1 if setup failed */ +static ssize_t colo_proxy_primary_setup(NetFilterState *nf) +{ +ColoProxyState *s = FILTER_COLO_PROXY(nf); +int sock; +sock = inet_connect(s->addr, NULL); +if (sock < 0) { +printf("colo proxy connect failed\n"); +g_free(s->addr); +return -1; +} +DEBUG("colo proxy connect success\n"); +s->sockfd = sock; + /* TODO: handle the packets that secondary forward */ +g_free(s->addr); As above comment. +return 1; +} + +/* Return 1 on success, or return -1 if setup failed */ +static ssize_t colo_proxy_secondary_setup(NetFilterState *nf) +{ +ColoProxyState *s = FILTER_COLO_PROXY(nf); Space~ fix +return colo_start_incoming(s); +} static void colo_proxy_setup(NetFilterState *nf, Error **errp) { ColoProxyState *s = FILTER_COLO_PROXY(nf); +ssize_t ret = 0; Space~ fix if (!s->addr) { error_setg(errp, "filter colo_proxy needs 'addr' \ property set!"); @@ -68,17 +141,29 @@ static void colo_proxy_setup(NetFilterState *nf, Error **errp) s->sockfd = -1; s->has_failover = false; colo_do_checkpoint = false; +s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf); +s->unprocessed_packets = g_hash_table_new_full(connection_key_hash, + connection_key_equal, + g_free, + connection_destroy); g_queue_init(&s->unprocessed_connections); if (!strcmp(mode, PRIMARY_MODE)) { s->colo_mode = COLO_PRIMARY_MODE; +ret = colo_proxy_primary_setup(nf); } else if (!strcmp(mode, SECONDARY_MODE)) { s->colo_mode = COLO_SECONDARY_MODE; +ret = colo_proxy_secondary_setup(nf); } else { error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "mode", "primary or secondary"); return; } +if (ret) { +DEBUG("colo_proxy_setup success\n"); +} else { +DEBUG("colo_proxy_setup failed\n"); +} } static void colo_proxy_class_init(ObjectClass *oc, void *data) diff --git a/net/colo-proxy.h b/net/col
Re: [Qemu-devel] [PATCH 01/40] 9pfs: allocate pdus with g_malloc/g_free
On Mon, 11/30 10:27, Fam Zheng wrote: > On Tue, 11/24 19:00, Paolo Bonzini wrote: > OK, and I think handle_9p_output no longer needs to check the returned > pointer. Yes it's removed in patch 3, good. Fam
Re: [Qemu-devel] [PATCH 01/40] 9pfs: allocate pdus with g_malloc/g_free
On Tue, 11/24 19:00, Paolo Bonzini wrote: > Prepare for moving the allocation to virtqueue_pop. > > Signed-off-by: Paolo Bonzini > --- > hw/9pfs/virtio-9p-device.c | 7 +-- > hw/9pfs/virtio-9p.c| 10 +++--- > hw/9pfs/virtio-9p.h| 2 -- > 3 files changed, 4 insertions(+), 15 deletions(-) > > diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c > index e3abcfa..72a93c2 100644 > --- a/hw/9pfs/virtio-9p-device.c > +++ b/hw/9pfs/virtio-9p-device.c > @@ -57,7 +57,7 @@ static void virtio_9p_device_realize(DeviceState *dev, > Error **errp) > { > VirtIODevice *vdev = VIRTIO_DEVICE(dev); > V9fsState *s = VIRTIO_9P(dev); > -int i, len; > +int len; > struct stat stat; > FsDriverEntry *fse; > V9fsPath path; > @@ -65,12 +65,7 @@ static void virtio_9p_device_realize(DeviceState *dev, > Error **errp) > virtio_init(vdev, "virtio-9p", VIRTIO_ID_9P, > sizeof(struct virtio_9p_config) + MAX_TAG_LEN); > > -/* initialize pdu allocator */ > -QLIST_INIT(&s->free_list); > QLIST_INIT(&s->active_list); > -for (i = 0; i < (MAX_REQ - 1); i++) { > -QLIST_INSERT_HEAD(&s->free_list, &s->pdus[i], next); > -} > > s->vq = virtio_add_queue(vdev, MAX_REQ, handle_9p_output); > > diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c > index f972731..747306b 100644 > --- a/hw/9pfs/virtio-9p.c > +++ b/hw/9pfs/virtio-9p.c > @@ -565,13 +565,9 @@ static int fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp, > V9fsQID *qidp) > > static V9fsPDU *alloc_pdu(V9fsState *s) > { > -V9fsPDU *pdu = NULL; > +V9fsPDU *pdu = g_new(V9fsPDU, 1); > > -if (!QLIST_EMPTY(&s->free_list)) { > -pdu = QLIST_FIRST(&s->free_list); > -QLIST_REMOVE(pdu, next); > -QLIST_INSERT_HEAD(&s->active_list, pdu, next); > -} > +QLIST_INSERT_HEAD(&s->active_list, pdu, next); > return pdu; > } OK, and I think handle_9p_output no longer needs to check the returned pointer. Fam > > @@ -584,8 +580,8 @@ static void free_pdu(V9fsState *s, V9fsPDU *pdu) > */ > if (!pdu->cancelled) { > QLIST_REMOVE(pdu, next); > -QLIST_INSERT_HEAD(&s->free_list, pdu, next); > } > +g_free(pdu); > } > } > > diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h > index d7a4dc1..1fb4ff9 100644 > --- a/hw/9pfs/virtio-9p.h > +++ b/hw/9pfs/virtio-9p.h > @@ -201,8 +201,6 @@ typedef struct V9fsState > { > VirtIODevice parent_obj; > VirtQueue *vq; > -V9fsPDU pdus[MAX_REQ]; > -QLIST_HEAD(, V9fsPDU) free_list; > QLIST_HEAD(, V9fsPDU) active_list; > V9fsFidState *fid_list; > FileOperations *ops; > -- > 1.8.3.1 > > >
Re: [Qemu-devel] [RFC PATCH 3/9] colo-proxy: add colo-proxy framework
On 11/28/2015 10:46 AM, Hailiang Zhang wrote: On 2015/11/27 20:27, Zhang Chen wrote: From: zhangchen Colo-proxy is a plugin of qemu netfilter like filter-buffer and dump Signed-off-by: zhangchen --- net/Makefile.objs | 1 + net/colo-proxy.c | 139 ++ net/colo-proxy.h | 63 + 3 files changed, 203 insertions(+) create mode 100644 net/colo-proxy.c create mode 100644 net/colo-proxy.h diff --git a/net/Makefile.objs b/net/Makefile.objs index 5fa2f97..95670f2 100644 --- a/net/Makefile.objs +++ b/net/Makefile.objs @@ -15,3 +15,4 @@ common-obj-$(CONFIG_VDE) += vde.o common-obj-$(CONFIG_NETMAP) += netmap.o common-obj-y += filter.o common-obj-y += filter-buffer.o +common-obj-y += colo-proxy.o diff --git a/net/colo-proxy.c b/net/colo-proxy.c new file mode 100644 index 000..98c2699 --- /dev/null +++ b/net/colo-proxy.c @@ -0,0 +1,139 @@ +/* + * COarse-grain LOck-stepping Virtual Machines for Non-stop Service (COLO) + * (a.k.a. Fault Tolerance or Continuous Replication) + * + * Copyright (c) 2015 HUAWEI TECHNOLOGIES CO., LTD. + * Copyright (c) 2015 FUJITSU LIMITED + * Copyright (c) 2015 Intel Corporation + * + * Author: Zhang Chen + * + * This work is licensed under the terms of the GNU GPL, version 2 or + * later. See the COPYING file in the top-level directory. + */ + +#include "colo-proxy.h" + +#define __DEBUG__ + +#ifdef __DEBUG__ +#define DEBUG(format, ...) printf(format, ##__VA_ARGS__) +#else +#define DEBUG(format, ...) +#endif + + +static ssize_t colo_proxy_receive_iov(NetFilterState *nf, + NetClientState *sender, + unsigned flags, + const struct iovec *iov, + int iovcnt, + NetPacketSent *sent_cb) +{ +/* + * We return size when buffer a packet, the sender will take it as + * a already sent packet, so sent_cb should not be called later. + * + */ +ColoProxyState *s = FILTER_COLO_PROXY(nf); blank space ~ fix thanks for review zhang chen +if (s->colo_mode == COLO_PRIMARY_MODE) { + /* colo_proxy_primary_handler */ +} else { + /* colo_proxy_primary_handler */ +} +return iov_size(iov, iovcnt); +} + +static void colo_proxy_cleanup(NetFilterState *nf) +{ + /* cleanup */ +} + + +static void colo_proxy_setup(NetFilterState *nf, Error **errp) +{ +ColoProxyState *s = FILTER_COLO_PROXY(nf); blank space ~ fix +if (!s->addr) { +error_setg(errp, "filter colo_proxy needs 'addr' \ + property set!"); +return; +} + +if (nf->direction != NET_FILTER_DIRECTION_ALL) { +printf("colo need queue all packet,\ +please startup colo-proxy with queue=all\n"); printf/error_setg/g fix +return; +} + +s->sockfd = -1; +s->has_failover = false; +colo_do_checkpoint = false; +g_queue_init(&s->unprocessed_connections); + +if (!strcmp(mode, PRIMARY_MODE)) { +s->colo_mode = COLO_PRIMARY_MODE; +} else if (!strcmp(mode, SECONDARY_MODE)) { +s->colo_mode = COLO_SECONDARY_MODE; +} else { +error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "mode", +"primary or secondary"); +return; +} +} + +static void colo_proxy_class_init(ObjectClass *oc, void *data) +{ +NetFilterClass *nfc = NETFILTER_CLASS(oc); + +nfc->setup = colo_proxy_setup; +nfc->cleanup = colo_proxy_cleanup; +nfc->receive_iov = colo_proxy_receive_iov; +} + +static char *colo_proxy_get_mode(Object *obj, Error **errp) +{ +return g_strdup(mode); +} Wrong patch ? Where did you define 'mode' ? sorry,in next patch,i will move define to this patch in next version. + +static void colo_proxy_set_mode(Object *obj, const char *value, Error **errp) +{ +g_free(mode); +mode = g_strdup(value); +} + +static char *colo_proxy_get_addr(Object *obj, Error **errp) +{ +ColoProxyState *s = FILTER_COLO_PROXY(obj); + +return g_strdup(s->addr); +} + +static void colo_proxy_set_addr(Object *obj, const char *value, Error **errp) +{ +ColoProxyState *s = FILTER_COLO_PROXY(obj); +g_free(s->addr); +s->addr = g_strdup(value); +} + +static void colo_proxy_init(Object *obj) +{ +object_property_add_str(obj, "mode", colo_proxy_get_mode, +colo_proxy_set_mode, NULL); +object_property_add_str(obj, "addr", colo_proxy_get_addr, +colo_proxy_set_addr, NULL); +} + +static const TypeInfo colo_proxy_info = { +.name = TYPE_FILTER_COLO_PROXY, +.parent = TYPE_NETFILTER, +.class_init = colo_proxy_class_init, +.instance_init = colo_proxy_init, +.instance_size = sizeof(ColoProxyState), +}; + +static void register_types(void) +{ +type_register_static(
Re: [Qemu-devel] [PATCH v2 3/8] dump-guest-memory: add basic "detach" support.
On Sat, 11/28 13:51, Peter Xu wrote: > On Fri, Nov 27, 2015 at 01:14:25PM +0800, Fam Zheng wrote: > > On Fri, 11/27 10:48, Peter Xu wrote: > > [snip] > > > > > This patch doesn't handle the incoming migration case, i.e. when QEMU is > > started with "-incoming", or "-incoming defer". Dump can go wrong when > > incoming > > migration happens at the same time. > > Sorry to missed these lines. Still not understand why this patch > cannot handle this if with [1] (Please check below for [1])? You're right, that does work. I missed that "defer" also sets RUN_STATE_INMIGRATE. Fam
Re: [Qemu-devel] [PATCH COLO-Frame v11 34/39] net/filter-buffer: Add default filter-buffer for each netdev
On 11/24/2015 05:25 PM, zhanghailiang wrote: We add each netdev a default filter-buffer, which will be used for COLO or Micro-checkpoint to buffer VM's packets. The name of default filter-buffer is 'nop'. For the default filter-buffer, it will not buffer any packets in default. So it has no side effect for the netdev. Signed-off-by: zhanghailiang Cc: Jason Wang Cc: Yang Hongyang --- v11: - New patch --- include/net/filter.h | 3 +++ net/filter-buffer.c | 74 net/net.c| 8 ++ 3 files changed, 85 insertions(+) diff --git a/include/net/filter.h b/include/net/filter.h index 2deda36..01a7e90 100644 --- a/include/net/filter.h +++ b/include/net/filter.h @@ -74,4 +74,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender, int iovcnt, void *opaque); +void netdev_add_default_filter_buffer(const char *netdev_id, + NetFilterDirection direction, + Error **errp); #endif /* QEMU_NET_FILTER_H */ diff --git a/net/filter-buffer.c b/net/filter-buffer.c index 57be149..195af68 100644 --- a/net/filter-buffer.c +++ b/net/filter-buffer.c @@ -14,6 +14,12 @@ #include "qapi/qmp/qerror.h" #include "qapi-visit.h" #include "qom/object.h" +#include "net/net.h" +#include "qapi/qmp/qdict.h" +#include "qapi/qmp-output-visitor.h" +#include "qapi/qmp-input-visitor.h" +#include "monitor/monitor.h" +#include "qmp-commands.h" #define TYPE_FILTER_BUFFER "filter-buffer" @@ -26,6 +32,8 @@ typedef struct FilterBufferState { NetQueue *incoming_queue; uint32_t interval; QEMUTimer release_timer; +bool is_default; +bool enable_buffer; } FilterBufferState; static void filter_buffer_flush(NetFilterState *nf) @@ -65,6 +73,10 @@ static ssize_t filter_buffer_receive_iov(NetFilterState *nf, { FilterBufferState *s = FILTER_BUFFER(nf); +/* Don't buffer any packets if the filter is not enabled */ +if (!s->enable_buffer) { +return 0; +} /* * We return size when buffer a packet, the sender will take it as * a already sent packet, so sent_cb should not be called later. @@ -102,6 +114,7 @@ static void filter_buffer_cleanup(NetFilterState *nf) static void filter_buffer_setup(NetFilterState *nf, Error **errp) { FilterBufferState *s = FILTER_BUFFER(nf); +char *path = object_get_canonical_path_component(OBJECT(nf)); /* * We may want to accept zero interval when VM FT solutions like MC @@ -114,6 +127,7 @@ static void filter_buffer_setup(NetFilterState *nf, Error **errp) } s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf); +s->is_default = !strcmp(path, "nop"); if (s->interval) { timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL, filter_buffer_release_timer, nf); @@ -163,6 +177,66 @@ out: error_propagate(errp, local_err); } +/* +* This will be used by COLO or MC FT, for which they will need +* to buffer the packets of VM's net devices, Here we add a default +* buffer filter for each netdev. The name of default buffer filter is +* 'nop' +*/ +void netdev_add_default_filter_buffer(const char *netdev_id, + NetFilterDirection direction, + Error **errp) +{ +QmpOutputVisitor *qov; +QmpInputVisitor *qiv; +Visitor *ov, *iv; +QObject *obj = NULL; +QDict *qdict; +void *dummy = NULL; +const char *id = "nop"; +char *queue = g_strdup(NetFilterDirection_lookup[direction]); +NetClientState *nc = qemu_find_netdev(netdev_id); +Error *err = NULL; + +/* FIXME: Not support multiple queues */ +if (!nc || nc->queue_index > 1) { queue memory leaks ? Thanks Li +return; +} +qov = qmp_output_visitor_new(); +ov = qmp_output_get_visitor(qov); +visit_start_struct(ov, &dummy, NULL, NULL, 0, &err); +if (err) { +goto out; +} +visit_type_str(ov, &nc->name, "netdev", &err); +if (err) { +goto out; +} +visit_type_str(ov, &queue, "queue", &err); +if (err) { +goto out; +} +visit_end_struct(ov, &err); +if (err) { +goto out; +} +obj = qmp_output_get_qobject(qov); +g_assert(obj != NULL); +qdict = qobject_to_qdict(obj); +qmp_output_visitor_cleanup(qov); + +qiv = qmp_input_visitor_new(obj); +iv = qmp_input_get_visitor(qiv); +object_add(TYPE_FILTER_BUFFER, id, qdict, iv, &err); +qmp_input_visitor_cleanup(qiv); +qobject_decref(obj); +out: +g_free(queue); +if (err) { +error_propagate(errp, err); +} +} + static void filter_buffer_init(Object *obj) { object_property_add(obj, "interval", "int", diff --git a/net/net.c b/net/net.c index ade6051..b36d49f 100644 ---
Re: [Qemu-devel] [PATCH v4 3/3] i.MX: Add an i.MX25 specific CCM class/instance.
Le 27/11/2015 21:26, Peter Crosthwaite a écrit : On Fri, Nov 27, 2015 at 11:54 AM, Jean-Christophe DUBOIS wrote: Le 27/11/2015 03:39, Peter Crosthwaite a écrit : On Wed, Nov 25, 2015 at 11:16 PM, Jean-Christophe Dubois wrote: Signed-off-by: Jean-Christophe Dubois This seems to slow down boot performance for i.MX25 Linux. Admittedly, the issue looks to be in timeout code for an unmodelled periph (NAND): [ cut here ] WARNING: CPU: 0 PID: 1 at /home/pcrost/poky/build/tmp/work-shared/qemuarmv5imx/kernel-source/drivers/mtd/nand/mxc_nand.c:464 wait_op_done+0xf0/0x114() timeout! useirq=0 Modules linked in: CPU: 0 PID: 1 Comm: swapper Not tainted 4.2.1 #1 Hardware name: Freescale i.MX25 (Device Tree Support) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (warn_slowpath_common+0x74/0xac) [] (warn_slowpath_common) from [] (warn_slowpath_fmt+0x30/0x40) [] (warn_slowpath_fmt) from [] (wait_op_done+0xf0/0x114) [] (wait_op_done) from [] (nand_scan_ident+0xdc/0x1560) [] (nand_scan_ident) from [] (mxcnd_probe+0x378/0x5c0) [] (mxcnd_probe) from [] (platform_drv_probe+0x44/0xac) [] (platform_drv_probe) from [] (driver_probe_device+0x180/0x2c4) [] (driver_probe_device) from [] (__driver_attach+0x8c/0x90) [] (__driver_attach) from [] (bus_for_each_dev+0x70/0xa0) [] (bus_for_each_dev) from [] (bus_add_driver+0x188/0x210) [] (bus_add_driver) from [] (driver_register+0x78/0xf8) [] (driver_register) from [] (do_one_initcall+0x84/0x1f0) [] (do_one_initcall) from [] (kernel_init_freeable+0x108/0x1c8) [] (kernel_init_freeable) from [] (kernel_init+0x8/0xec) [] (kernel_init) from [] (ret_from_fork+0x14/0x34) ---[ end trace 13248cb1a1bbcb9c ]--- <> nand: No NAND device found ... Without this patch, the delay is around 2 seconds, with this patch it is 10+. Any idea what would cause it? Are you removing the NAND from DTS for your testing and do we not care about these errors paths? Regards, Peter The kernel I am testing with is 3.19.0 but without without DTS tree. Linux version 3.19.0 (jcd@jcd-U31SG) (gcc version 4.6.3 (GCC) ) #2 Mon Jun 22 00:32:04 CEST 2015 So I am not up to date on this side and I might not test the same devices as you do as I generated a "minimal" kernel for my test. So the DTB+defconfig boot is actually in pretty good shape. That NAND thing is the only real bootlog issue. All other missing peripherals fail gracefully. Anyway, testing the timer code I found that running "sleep 60" on both PTF doesn't give the expected 60 seconds in "real world time": On i.MX31 (using i.MX31 CCM) => 47 seconds On i.MX25 (using i.MX31 CCM) => 52 seconds (before change. close enough?) Confirmed this result, exactly the same here. The 47 sec for i.MX31 is because Qemu sets the IPG/GPT clock to 50 MHz but the Linux guest code for some reason detects only a 39 MHz GPT clock. The decrepancy explains that the time then goes faster for the Linux guest (sleep 60 wait in fact for 47 real seconds). I need to find why Linux is not retrieving the "correct" freq value from Qemu. JC On i.MX25 (using i.MX25 CCM) => 80 seconds Another indication, the bogomips: On i.MX31 (using i.MX31 CCM) => 78 On i.MX25 (using i.MX31 CCM) => 87 (before change. close enough?) On i.MX25 (using i.MX25 CCM) => 133 I wouldn't worry about this, this is rarely accurate in QEMU. Regards, Peter So, yes, for some reason "time goes slower" after switching to i.MX25 CCM ... (but it was also going too fast before with i.MX31 CCM) As the CCM doesn't really provide any clock (just a clock value) something must not be right in the way the i.MX GPT timer is computing time. I need to look after this. JC --- Changes since v1: * rework loging to match other i.MX drivers Changes since v2: * We moved to an inheritance QOM scheme Changes since v3: * Rework logging based on comments. hw/arm/fsl-imx25.c | 2 +- hw/misc/Makefile.objs | 1 + hw/misc/imx25_ccm.c | 276 include/hw/arm/fsl-imx25.h | 4 +- include/hw/misc/imx25_ccm.h | 59 ++ 5 files changed, 339 insertions(+), 3 deletions(-) create mode 100644 hw/misc/imx25_ccm.c create mode 100644 include/hw/misc/imx25_ccm.h
Re: [Qemu-devel] [PATCH v1 2/2] arm: fsl-imx25: Add SD support
Hello Peter, Le 28/11/2015 21:50, Peter Crosthwaite a écrit : Add the two SD card controllers as the generic sysbus SDHCI device. Tested as sucessfully working with Linux 4.2. using the SD card for the root file system. I guess the SDHCI device present in Qemu is (surprisingly) close enough to the i.MX version so that it works with a "generic" guest driver not using all device features which is nice. But I am afraid the Freescale version is not 100% compatible. To me it looks like some registers are different or not supported in i.MX25. I guess we could "fix it" later when we meet some different guest driver. JC Signed-off-by: Peter Crosthwaite --- hw/arm/fsl-imx25.c | 26 ++ include/hw/arm/fsl-imx25.h | 9 + 2 files changed, 35 insertions(+) diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c index e1cadac..9ccbd03 100644 --- a/hw/arm/fsl-imx25.c +++ b/hw/arm/fsl-imx25.c @@ -68,6 +68,11 @@ static void fsl_imx25_init(Object *obj) object_initialize(&s->gpio[i], sizeof(s->gpio[i]), TYPE_IMX_GPIO); qdev_set_parent_bus(DEVICE(&s->gpio[i]), sysbus_get_default()); } + +for (i = 0; i < FSL_IMX25_NUM_ESDHCS; i++) { +object_initialize(&s->esdhc[i], sizeof(s->esdhc[i]), TYPE_SYSBUS_SDHCI); +qdev_set_parent_bus(DEVICE(&s->esdhc[i]), sysbus_get_default()); +} } static void fsl_imx25_realize(DeviceState *dev, Error **errp) @@ -243,6 +248,27 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp) gpio_table[i].irq)); } +/* Initialize all eSDHCs */ +for (i = 0; i < FSL_IMX25_NUM_ESDHCS; i++) { +static const struct { +hwaddr addr; +unsigned int irq; +} esdhc_table[FSL_IMX25_NUM_ESDHCS] = { +{ FSL_IMX25_ESDHC1_ADDR, FSL_IMX25_ESDHC1_IRQ }, +{ FSL_IMX25_ESDHC2_ADDR, FSL_IMX25_ESDHC2_IRQ } +}; + +object_property_set_bool(OBJECT(&s->esdhc[i]), true, "realized", &err); +if (err) { +error_propagate(errp, err); +return; +} +sysbus_mmio_map(SYS_BUS_DEVICE(&s->esdhc[i]), 0, esdhc_table[i].addr); +sysbus_connect_irq(SYS_BUS_DEVICE(&s->esdhc[i]), 0, + qdev_get_gpio_in(DEVICE(&s->avic), +esdhc_table[i].irq)); +} + /* initialize 2 x 16 KB ROM */ memory_region_init_rom_device(&s->rom[0], NULL, NULL, NULL, "imx25.rom0", FSL_IMX25_ROM0_SIZE, &err); diff --git a/include/hw/arm/fsl-imx25.h b/include/hw/arm/fsl-imx25.h index 73f50c6..ade4d42 100644 --- a/include/hw/arm/fsl-imx25.h +++ b/include/hw/arm/fsl-imx25.h @@ -26,6 +26,7 @@ #include "hw/net/imx_fec.h" #include "hw/i2c/imx_i2c.h" #include "hw/gpio/imx_gpio.h" +#include "hw/sd/sdhci.h" #include "exec/memory.h" #define TYPE_FSL_IMX25 "fsl,imx25" @@ -36,6 +37,7 @@ #define FSL_IMX25_NUM_EPITS 2 #define FSL_IMX25_NUM_I2CS 3 #define FSL_IMX25_NUM_GPIOS 4 +#define FSL_IMX25_NUM_ESDHCS 2 typedef struct FslIMX25State { /*< private >*/ @@ -51,6 +53,7 @@ typedef struct FslIMX25State { IMXFECStatefec; IMXI2CStatei2c[FSL_IMX25_NUM_I2CS]; IMXGPIOState gpio[FSL_IMX25_NUM_GPIOS]; +SDHCIState esdhc[2]; MemoryRegion rom[2]; MemoryRegion iram; MemoryRegion iram_alias; @@ -211,6 +214,10 @@ typedef struct FslIMX25State { #define FSL_IMX25_GPIO4_SIZE0x4000 #define FSL_IMX25_GPIO3_ADDR0x53FA4000 #define FSL_IMX25_GPIO3_SIZE0x4000 +#define FSL_IMX25_ESDHC1_ADDR 0x53FB4000 +#define FSL_IMX25_ESDHC1_SIZE 0x4000 +#define FSL_IMX25_ESDHC2_ADDR 0x53FBB000 +#define FSL_IMX25_ESDHC2_SIZE 0x4000 #define FSL_IMX25_GPIO1_ADDR0x53FCC000 #define FSL_IMX25_GPIO1_SIZE0x4000 #define FSL_IMX25_GPIO2_ADDR0x53FD @@ -245,5 +252,7 @@ typedef struct FslIMX25State { #define FSL_IMX25_GPIO2_IRQ 51 #define FSL_IMX25_GPIO3_IRQ 16 #define FSL_IMX25_GPIO4_IRQ 23 +#define FSL_IMX25_ESDHC1_IRQ9 +#define FSL_IMX25_ESDHC2_IRQ8 #endif /* FSL_IMX25_H */
[Qemu-devel] [Bug 1481272] Re: main-loop: WARNING: I/O thread spun for 1000 iterations
I have experienced this behavior (main-loop: WARNING: I/O thread spun for 1000 iterations) and the resulting degraded performance. The VM becomes very unresponsive but eventually recovers. My setup: Ubuntu 15.10 | qemu-system-x86_64 --version QEMU emulator version 2.3.0 (Debian 1:2.3+dfsg-5ubuntu9) HW = Dell Precision M6600 with Intel(R) Core(TM) i7-2760QM CPU I agree with other comments that it seems related to high disk I/O as the problem seems to occur during VM install or other VM operations which read/write large amounts of data. -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1481272 Title: main-loop: WARNING: I/O thread spun for 1000 iterations Status in QEMU: New Bug description: I compile the latest qemu to launch a VM but the monitor output the "main-loop: WARNING: I/O thread spun for 1000 iterations". # /usr/local/bin/qemu-system-x86_64 -name rhel6 -S -no-kvm -m 1024M -realtime mlock=off -smp 1,sockets=1,cores=1,threads=1 -uuid c9dd2a5c-40f2-fd3d-3c54-9cd84f8b9174 -rtc base=utc -drive file=/home/samba-share/ubuntu.img,if=none,id=drive-virtio-disk0,format=qcow2,cache=none -device virtio-blk-pci,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=disk,serial=425618d4-871f-4021-bc5d-bcd7f1b5ca9c,bootindex=0 -vnc :1 -boot menu=on -monitor stdio QEMU 2.3.93 monitor - type 'help' for more information (qemu) c (qemu) main-loop: WARNING: I/O thread spun for 1000 iterations <--- qemu]# git branch -v * master e95edef Merge remote-tracking branch 'remotes/sstabellini/tags/xen-migration-2.4-tag' into staging To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1481272/+subscriptions
Re: [Qemu-devel] [PATCH 1/4] vmxnet3: The vmxnet3 device is a PCIE endpoint
Hi, On Wed, 25 Nov 2015 16:24:39 +0800 Jason Wang wrote: > >>> @@ -2568,6 +2572,7 @@ static void vmxnet3_class_init(ObjectClass *class, > >>> void *data) > >>> c->class_id = PCI_CLASS_NETWORK_ETHERNET; > >>> c->subsystem_vendor_id = PCI_VENDOR_ID_VMWARE; > >>> c->subsystem_id = PCI_DEVICE_ID_VMWARE_VMXNET3; > >>> +c->is_express = 1; > >> Should we do this conditionally? And how about the migration > >> compatibility? Looks like pcie device is using vmstate_pcie_device > >> instead of vmstate_pci_device, maybe need a new property bit for this. > > (Responding for the entire series) > > > > Agreed. Will limit these changes for new versions. > > > > What's your suggested plan? > > Does it make sense to have a property for each change (as they are not > > necessarily related), or is it too tedious and one property will suffice? > > Since they are not necessarily related, we'd better use a property for > each change. Would it make sense if we expose a new vmxnet3 type to differenciate pcie vs pci instances of vmxnet3? Otherwise, migration gets more complicated, as we need to use either vmstate_pci_device or vmstate_pcie_device; also, upon vm load, we need to preserve the semantics saved (whether the instance was pci or pcie). I have managed to do so, but is a bit tedious; Exposing a new type seems cleaner.
Re: [Qemu-devel] [PATCH] m25p80.c Added support for N25Q256 and N25Q512
On Sun, Nov 29, 2015 at 5:12 AM, Krzeminski, Marcin (Nokia - PL/Wroclaw) wrote: > > >> -Original Message- >> From: EXT Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com] >> Sent: Saturday, November 28, 2015 7:50 PM >> To: Krzeminski, Marcin (Nokia - PL/Wroclaw) >> Cc: qemu-devel@nongnu.org; g...@xilinx.com; Sai Pavan Boddu >> Subject: Re: [Qemu-devel] [PATCH] m25p80.c Added support for N25Q256 >> and N25Q512 >> >> These features are also available in Xilinx QEMU if you want to compare >> implementation: >> >> https://github.com/Xilinx/qemu/blob/pub/2015.2.plnx/hw/block/m25p80.c >> >> That work also handles the larger and newer Spansion flash parts, as well as >> the quad and dual mode commands for QSPI (also features of n25qXXX). >> > Too bad I did not checked xilix fork, so V2 of this patch does not make sense > since all is already implemented. V2 still makes sense. My V1 comments are pretty minor and that Xilinx work will need cleanup before upstreaming. It is not a competing implementaiton and the diff there will go down when it is rebased on yours. > Why didn't xilinks merge it with mainline? There's a lot in that tree and Xilinx hasn't gotten around to it yet. > Anyway, I do not like not commented reviews, so lets play the game... > >> On Sat, Nov 28, 2015 at 9:06 AM, Krzeminski, Marcin (Nokia - >> PL/Wroclaw) wrote: >> > It is my first patch, so any comment are really welcome. >> > >> Check MAINTAINERS file for relevant people to CC. >> >> To make informal comments on your patches, you but them below the line ... >> >> > Changes: >> > * Removed unused variable >> > * Added support for n25q256a and n25q512a >> > * Added support for 4bytes address mode >> >> 4 byte addressing is a feature common to more than just n25qXXX. It should >> be done as a separate prepended patch. >> >> > * Added support for banked read mode >> > * Added support for sw reset flash commands >> > * Added Read Flag Status register command support >> > >> >> Basically these bullets should indicate separate patches to ease review. >> >> > Signed-off-by: Marcin Krzeminski >> > --- >> >> ... here. So when the maintainers apply the patch they are not committed to >> the git logs. >> > Thanks. >> > hw/block/m25p80.c | 94 >> > +++ >> > 1 file changed, 88 insertions(+), 6 deletions(-) >> > >> > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index >> > efc43dd..c8b92d8 100644 >> > --- a/hw/block/m25p80.c >> > +++ b/hw/block/m25p80.c >> > @@ -47,6 +47,9 @@ >> > */ >> > #define WR_1 0x100 >> > >> > +/* 16 MiB max in 3 byte address mode */ #define MAX_3BYTES_SIZE >> > +0x100 >> > + >> > typedef struct FlashPartInfo { >> > const char *part_name; >> > /* jedec code. (jedec >> 16) & 0xff is the 1st byte, >> 8 the 2nd >> > etc */ @@ -206,6 +209,8 @@ static const FlashPartInfo known_devices[] >> > = { >> > >> > /* Numonyx -- n25q128 */ >> > { INFO("n25q128", 0x20ba18, 0, 64 << 10, 256, 0) }, >> > +{ INFO("n25q256a", 0x20ba19, 0, 64 << 10, 512, ER_4K) }, >> > +{ INFO("n25q512a", 0x20ba20, 0, 64 << 10, 1024, ER_4K) }, >> > }; >> > >> > typedef enum { >> > @@ -216,6 +221,7 @@ typedef enum { >> > WREN = 0x6, >> > JEDEC_READ = 0x9f, >> > BULK_ERASE = 0xc7, >> > +READ_FSL = 0x70, >> >> Where does "FSL" come from? I am looking at an n25q256 datasheet here >> that has this is "RFSR". >> >> http://datasheet.octopart.com/N25Q256A13E1241F-Micron-datasheet- >> 11552757.pdf >> >> Admittedly, the vendors do tend to rename this stuff from part-to-part. To >> keep consistent with surrounding code, this would be READ_FSR. > I seem it is a typo, it should be READ_FSR. >> >> > >> > READ = 0x3, >> > FAST_READ = 0xb, >> > @@ -231,6 +237,15 @@ typedef enum { >> > ERASE_4K = 0x20, >> > ERASE_32K = 0x52, >> > ERASE_SECTOR = 0xd8, >> > + >> > +ENTER_4BYTE_ADDR_MODE = 0xB7, >> > +LEAVE_4BYTE_ADDR_MODE = 0xE9, >> > + >> > +EXTEND_ADDR_READ = 0xC8, >> > +EXTEND_ADDR_WRITE = 0xC5, >> > + >> >> Same data sheet has: WREAR, RDEAR, EN4BYTEADDR ... but Xilinx code has >> something different again. In both cases, it is shorter, so I think this >> should >> just be something shorter. >> > True. >> > +RESET_ENABLE = 0x66, >> > +RESET_MEMORY = 0x99, >> > } FlashCMD; >> > >> > typedef enum { >> > @@ -244,8 +259,6 @@ typedef enum { >> > typedef struct Flash { >> > SSISlave parent_obj; >> > >> > -uint32_t r; >> > - >> >> Even the trivial cleanup can be a separate patch. >> >> > BlockBackend *blk; >> > >> > uint8_t *storage; >> > @@ -260,6 +273,9 @@ typedef struct Flash { >> > uint8_t cmd_in_progress; >> > uint64_t cur_addr; >> > bool write_enable; >> > +bool four_bytes_address_mode; >> > +bool reset_enable; >> > +uint8_t extended_addr_reg; >> >> The datasheets abbreviate this to "EAR" so I think the same should be done >> in c
Re: [Qemu-devel] [PULL 07/15] q35: Check propery to determine if iommu is set
Hi Marcel, Marcel Apfelbaum writes: ... > > Maybe is too late, but this contradicts QEMU usage, as I understand Why late ? We can always revert it :) > object_property_get_* should be used when we don't know object's type. My understanding is that it's not mandatory to use it only when type is unknown. Ofcourse, it makes it redundant when you do know the type. I tend to follow convention, I noticed another call to qdev_get_machine, and so opted for this. I am actually ok either way and don't prefer one way over the other. > Why use "iommu" when you can simply call current_machine->iommu ? > (if you don't like the wrapper, which is pretty harmless in my opinion) > > Thanks, > Marcel > >> mch_init_dmar(mch); >> } >> } >>
Re: [Qemu-devel] [for-2.6 PATCH v2 1/2] pc: Remove redundant code from pc-*-2.3 machine classes
On 11/27/2015 08:01 PM, Eduardo Habkost wrote: Remove the redundant 'alias = NULL' and 'is_default = 0' lines from older machine-types. pc_*_2_4_machine_options() already clear those fields, so they don't need to be cleared by pc_*_2_3_machine_options(). Signed-off-by: Eduardo Habkost --- hw/i386/pc_piix.c | 2 -- hw/i386/pc_q35.c | 1 - 2 files changed, 3 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 2e41efe..1a4ff01 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -499,8 +499,6 @@ static void pc_i440fx_2_3_machine_options(MachineClass *m) { pc_i440fx_2_4_machine_options(m); m->hw_version = "2.3.0"; -m->alias = NULL; -m->is_default = 0; SET_MACHINE_COMPAT(m, PC_COMPAT_2_3); } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 133bc68..f17acca 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -399,7 +399,6 @@ static void pc_q35_2_3_machine_options(MachineClass *m) m->hw_version = "2.3.0"; m->no_floppy = 0; m->no_tco = 1; -m->alias = NULL; SET_MACHINE_COMPAT(m, PC_COMPAT_2_3); } Reviewed-by: Marcel Apfelbaum
Re: [Qemu-devel] [for-2.6 PATCH v2 2/2] pc: Add pc-*-2.6 machine classes
On 11/27/2015 08:01 PM, Eduardo Habkost wrote: Add pc-i440fx-2.6 and pc-q35-2.6 machine classes. Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Add missing backslash to PC_COMPAT_2_4 --- hw/i386/pc_piix.c| 16 +--- hw/i386/pc_q35.c | 13 +++-- include/hw/compat.h | 3 +++ include/hw/i386/pc.h | 4 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 1a4ff01..299c07f 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -469,13 +469,25 @@ static void pc_i440fx_machine_options(MachineClass *m) m->default_display = "std"; } -static void pc_i440fx_2_5_machine_options(MachineClass *m) +static void pc_i440fx_2_6_machine_options(MachineClass *m) { pc_i440fx_machine_options(m); m->alias = "pc"; m->is_default = 1; } +DEFINE_I440FX_MACHINE(v2_6, "pc-i440fx-2.6", NULL, + pc_i440fx_2_6_machine_options); + + +static void pc_i440fx_2_5_machine_options(MachineClass *m) +{ +pc_i440fx_2_6_machine_options(m); +m->alias = NULL; +m->is_default = 0; +SET_MACHINE_COMPAT(m, PC_COMPAT_2_5); +} + DEFINE_I440FX_MACHINE(v2_5, "pc-i440fx-2.5", NULL, pc_i440fx_2_5_machine_options); @@ -485,8 +497,6 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m) PCMachineClass *pcmc = PC_MACHINE_CLASS(m); pc_i440fx_2_5_machine_options(m); m->hw_version = "2.4.0"; -m->alias = NULL; -m->is_default = 0; pcmc->broken_reserved_end = true; SET_MACHINE_COMPAT(m, PC_COMPAT_2_4); } diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index f17acca..0086546 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -370,12 +370,22 @@ static void pc_q35_machine_options(MachineClass *m) m->no_tco = 0; } -static void pc_q35_2_5_machine_options(MachineClass *m) +static void pc_q35_2_6_machine_options(MachineClass *m) { pc_q35_machine_options(m); m->alias = "q35"; } +DEFINE_Q35_MACHINE(v2_6, "pc-q35-2.6", NULL, + pc_q35_2_6_machine_options); + +static void pc_q35_2_5_machine_options(MachineClass *m) +{ +pc_q35_2_6_machine_options(m); +m->alias = NULL; +SET_MACHINE_COMPAT(m, PC_COMPAT_2_5); +} + DEFINE_Q35_MACHINE(v2_5, "pc-q35-2.5", NULL, pc_q35_2_5_machine_options); @@ -384,7 +394,6 @@ static void pc_q35_2_4_machine_options(MachineClass *m) PCMachineClass *pcmc = PC_MACHINE_CLASS(m); pc_q35_2_5_machine_options(m); m->hw_version = "2.4.0"; -m->alias = NULL; pcmc->broken_reserved_end = true; SET_MACHINE_COMPAT(m, PC_COMPAT_2_4); } diff --git a/include/hw/compat.h b/include/hw/compat.h index d0b1c4f..fae0d8e 100644 --- a/include/hw/compat.h +++ b/include/hw/compat.h @@ -1,6 +1,9 @@ #ifndef HW_COMPAT_H #define HW_COMPAT_H +#define HW_COMPAT_2_5 \ +/* empty */ + #define HW_COMPAT_2_4 \ {\ .driver = "virtio-blk-device",\ diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 854c330..040d1f2 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -296,7 +296,11 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); int e820_get_num_entries(void); bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); +#define PC_COMPAT_2_5 \ +/* empty */ Hi, Should PC_COMPAT_2_5 have automatically HW_COMPAT_2_5 ? Thanks, Marcel + #define PC_COMPAT_2_4 \ +PC_COMPAT_2_5 \ HW_COMPAT_2_4 \ {\ .driver = "Haswell-" TYPE_X86_CPU,\
Re: [Qemu-devel] [PATCH v2 0/4] target-tilegx: Implement floating point instructions
Hello Maintainers: Please help check these patches when you have time. If it is necessary to send patch v3 for it, please let me know. Thanks. On 11/17/15 03:37, Chen Gang wrote: > From d0f0e0a78e81f9589d25b0a2b4ad826d6e55257d Mon Sep 17 00:00:00 2001 > From: Chen Gang > Date: Tue, 17 Nov 2015 03:13:54 +0800 > Subject: [PATCH v2 0/4] target-tilegx: Implement floating point instructions > > These patches are the normal floating point implementation, instead of > the original temporary one. > > It passes building, and gcc testsuite. > > Chen Gang (4): > target-tilegx: Add floating point shared functions > target-tilegx: Add single floating point implementation > target-tilegx: Add double floating point implementation > target-tilegx: Integrate floating pointer implementation > > target-tilegx/Makefile.objs | 3 +- > target-tilegx/cpu.h | 2 + > target-tilegx/helper-fdouble.c | 400 + > target-tilegx/helper-fshared.c | 53 ++ > target-tilegx/helper-fsingle.c | 212 ++ > target-tilegx/helper.h | 12 ++ > target-tilegx/translate.c | 68 ++- > 7 files changed, 740 insertions(+), 10 deletions(-) > create mode 100644 target-tilegx/helper-fdouble.c > create mode 100644 target-tilegx/helper-fshared.c > create mode 100644 target-tilegx/helper-fsingle.c > > -- > 1.9.3 > -- Chen Gang (陈刚) Open, share, and attitude like air, water, and life which God blessed
[Qemu-devel] [PATCH] ui/curses: Fix color attribute of monitor for curses
Current text_console_update() writes totally broken color attributes to console_write_ch(). The format now is writing, [WRONG] bold << 21 | fg << 12 | bg << 8 | char fg == 3bits curses color number bg == 3bits curses color number I can't see this format is where come from. Anyway, this doesn't work at all. What curses expects is actually (and vga.c is using), [RIGHT] bold << 21 | bg << 11 | fg << 8 | char fg == 3bits vga color number bg == 3bits vga color number And curses set COLOR_PAIR() up to match this format, and curses's chtype. I.e, bold | color_pair | char color_pair == (bg << 3 | fg) To fix, this simply uses VGA color number everywhere except curses.c internal. Then, convert it to above [RIGHT] format to write by console_write_ch(). And as bonus, this reduces to expose curses define to other parts (removes COLOR_* from console.c). [Tested the first line is displayed as white on blue back for monitor in curses console] Signed-off-by: OGAWA Hirofumi --- hw/display/jazz_led.c |6 +- hw/display/vga.c |3 - include/ui/console.h | 15 +++ ui/console.c | 101 + ui/curses.c | 13 -- 5 files changed, 76 insertions(+), 62 deletions(-) diff -puN include/ui/console.h~fix-curses-colors-for-monitor include/ui/console.h --- qemu/include/ui/console.h~fix-curses-colors-for-monitor 2015-11-29 21:49:56.561539144 +0900 +++ qemu-hirofumi/include/ui/console.h 2015-11-29 22:16:21.817985908 +0900 @@ -30,6 +30,21 @@ #define GUI_REFRESH_INTERVAL_DEFAULT30 #define GUI_REFRESH_INTERVAL_IDLE 3000 +/* Color number is match to standard vga palette */ +enum qemu_color_names { +QEMU_COLOR_BLACK = 0, +QEMU_COLOR_BLUE= 1, +QEMU_COLOR_GREEN = 2, +QEMU_COLOR_CYAN= 3, +QEMU_COLOR_RED = 4, +QEMU_COLOR_MAGENTA = 5, +QEMU_COLOR_YELLOW = 6, +QEMU_COLOR_WHITE = 7 +}; +/* Convert to curses char attributes */ +#define ATTR2CHTYPE(c, fg, bg, bold) \ +((bold) << 21 | (bg) << 11 | (fg) << 8 | (c)) + typedef void QEMUPutKBDEvent(void *opaque, int keycode); typedef void QEMUPutLEDEvent(void *opaque, int ledstate); typedef void QEMUPutMouseEvent(void *opaque, int dx, int dy, int dz, int buttons_state); diff -puN ui/console.c~fix-curses-colors-for-monitor ui/console.c --- qemu/ui/console.c~fix-curses-colors-for-monitor 2015-11-29 21:49:56.561539144 +0900 +++ qemu-hirofumi/ui/console.c 2015-11-29 22:19:02.477621279 +0900 @@ -375,42 +375,29 @@ static void vga_bitblt(QemuConsole *con, #include "vgafont.h" -#ifndef CONFIG_CURSES -enum color_names { -COLOR_BLACK = 0, -COLOR_RED = 1, -COLOR_GREEN = 2, -COLOR_YELLOW = 3, -COLOR_BLUE= 4, -COLOR_MAGENTA = 5, -COLOR_CYAN= 6, -COLOR_WHITE = 7 -}; -#endif - #define QEMU_RGB(r, g, b) \ { .red = r << 8, .green = g << 8, .blue = b << 8, .alpha = 0x } static const pixman_color_t color_table_rgb[2][8] = { { /* dark */ -QEMU_RGB(0x00, 0x00, 0x00), /* black */ -QEMU_RGB(0xaa, 0x00, 0x00), /* red */ -QEMU_RGB(0x00, 0xaa, 0x00), /* green */ -QEMU_RGB(0xaa, 0xaa, 0x00), /* yellow */ -QEMU_RGB(0x00, 0x00, 0xaa), /* blue */ -QEMU_RGB(0xaa, 0x00, 0xaa), /* magenta */ -QEMU_RGB(0x00, 0xaa, 0xaa), /* cyan */ -QEMU_RGB(0xaa, 0xaa, 0xaa), /* white */ +[QEMU_COLOR_BLACK] = QEMU_RGB(0x00, 0x00, 0x00), /* black */ +[QEMU_COLOR_BLUE]= QEMU_RGB(0x00, 0x00, 0xaa), /* blue */ +[QEMU_COLOR_GREEN] = QEMU_RGB(0x00, 0xaa, 0x00), /* green */ +[QEMU_COLOR_CYAN]= QEMU_RGB(0x00, 0xaa, 0xaa), /* cyan */ +[QEMU_COLOR_RED] = QEMU_RGB(0xaa, 0x00, 0x00), /* red */ +[QEMU_COLOR_MAGENTA] = QEMU_RGB(0xaa, 0x00, 0xaa), /* magenta */ +[QEMU_COLOR_YELLOW] = QEMU_RGB(0xaa, 0xaa, 0x00), /* yellow */ +[QEMU_COLOR_WHITE] = QEMU_RGB(0xaa, 0xaa, 0xaa), /* white */ }, { /* bright */ -QEMU_RGB(0x00, 0x00, 0x00), /* black */ -QEMU_RGB(0xff, 0x00, 0x00), /* red */ -QEMU_RGB(0x00, 0xff, 0x00), /* green */ -QEMU_RGB(0xff, 0xff, 0x00), /* yellow */ -QEMU_RGB(0x00, 0x00, 0xff), /* blue */ -QEMU_RGB(0xff, 0x00, 0xff), /* magenta */ -QEMU_RGB(0x00, 0xff, 0xff), /* cyan */ -QEMU_RGB(0xff, 0xff, 0xff), /* white */ +[QEMU_COLOR_BLACK] = QEMU_RGB(0x00, 0x00, 0x00), /* black */ +[QEMU_COLOR_BLUE]= QEMU_RGB(0x00, 0x00, 0xff), /* blue */ +[QEMU_COLOR_GREEN] = QEMU_RGB(0x00, 0xff, 0x00), /* green */ +[QEMU_COLOR_CYAN]= QEMU_RGB(0x00, 0xff, 0xff), /* cyan */ +[QEMU_COLOR_RED] = QEMU_RGB(0xff, 0x00, 0x00), /* red */ +[QEMU_COLOR_MAGENTA] = QEMU_RGB(0xff, 0x00, 0xff), /* magenta */ +[QEMU_C
Re: [Qemu-devel] [PATCH] m25p80.c Added support for N25Q256 and N25Q512
> -Original Message- > From: EXT Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com] > Sent: Saturday, November 28, 2015 7:50 PM > To: Krzeminski, Marcin (Nokia - PL/Wroclaw) > Cc: qemu-devel@nongnu.org; g...@xilinx.com; Sai Pavan Boddu > Subject: Re: [Qemu-devel] [PATCH] m25p80.c Added support for N25Q256 > and N25Q512 > > These features are also available in Xilinx QEMU if you want to compare > implementation: > > https://github.com/Xilinx/qemu/blob/pub/2015.2.plnx/hw/block/m25p80.c > > That work also handles the larger and newer Spansion flash parts, as well as > the quad and dual mode commands for QSPI (also features of n25qXXX). > Too bad I did not checked xilix fork, so V2 of this patch does not make sense since all is already implemented. Why didn't xilinks merge it with mainline? Anyway, I do not like not commented reviews, so lets play the game... > On Sat, Nov 28, 2015 at 9:06 AM, Krzeminski, Marcin (Nokia - > PL/Wroclaw) wrote: > > It is my first patch, so any comment are really welcome. > > > Check MAINTAINERS file for relevant people to CC. > > To make informal comments on your patches, you but them below the line ... > > > Changes: > > * Removed unused variable > > * Added support for n25q256a and n25q512a > > * Added support for 4bytes address mode > > 4 byte addressing is a feature common to more than just n25qXXX. It should > be done as a separate prepended patch. > > > * Added support for banked read mode > > * Added support for sw reset flash commands > > * Added Read Flag Status register command support > > > > Basically these bullets should indicate separate patches to ease review. > > > Signed-off-by: Marcin Krzeminski > > --- > > ... here. So when the maintainers apply the patch they are not committed to > the git logs. > Thanks. > > hw/block/m25p80.c | 94 > > +++ > > 1 file changed, 88 insertions(+), 6 deletions(-) > > > > diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index > > efc43dd..c8b92d8 100644 > > --- a/hw/block/m25p80.c > > +++ b/hw/block/m25p80.c > > @@ -47,6 +47,9 @@ > > */ > > #define WR_1 0x100 > > > > +/* 16 MiB max in 3 byte address mode */ #define MAX_3BYTES_SIZE > > +0x100 > > + > > typedef struct FlashPartInfo { > > const char *part_name; > > /* jedec code. (jedec >> 16) & 0xff is the 1st byte, >> 8 the 2nd > > etc */ @@ -206,6 +209,8 @@ static const FlashPartInfo known_devices[] > > = { > > > > /* Numonyx -- n25q128 */ > > { INFO("n25q128", 0x20ba18, 0, 64 << 10, 256, 0) }, > > +{ INFO("n25q256a", 0x20ba19, 0, 64 << 10, 512, ER_4K) }, > > +{ INFO("n25q512a", 0x20ba20, 0, 64 << 10, 1024, ER_4K) }, > > }; > > > > typedef enum { > > @@ -216,6 +221,7 @@ typedef enum { > > WREN = 0x6, > > JEDEC_READ = 0x9f, > > BULK_ERASE = 0xc7, > > +READ_FSL = 0x70, > > Where does "FSL" come from? I am looking at an n25q256 datasheet here > that has this is "RFSR". > > http://datasheet.octopart.com/N25Q256A13E1241F-Micron-datasheet- > 11552757.pdf > > Admittedly, the vendors do tend to rename this stuff from part-to-part. To > keep consistent with surrounding code, this would be READ_FSR. I seem it is a typo, it should be READ_FSR. > > > > > READ = 0x3, > > FAST_READ = 0xb, > > @@ -231,6 +237,15 @@ typedef enum { > > ERASE_4K = 0x20, > > ERASE_32K = 0x52, > > ERASE_SECTOR = 0xd8, > > + > > +ENTER_4BYTE_ADDR_MODE = 0xB7, > > +LEAVE_4BYTE_ADDR_MODE = 0xE9, > > + > > +EXTEND_ADDR_READ = 0xC8, > > +EXTEND_ADDR_WRITE = 0xC5, > > + > > Same data sheet has: WREAR, RDEAR, EN4BYTEADDR ... but Xilinx code has > something different again. In both cases, it is shorter, so I think this > should > just be something shorter. > True. > > +RESET_ENABLE = 0x66, > > +RESET_MEMORY = 0x99, > > } FlashCMD; > > > > typedef enum { > > @@ -244,8 +259,6 @@ typedef enum { > > typedef struct Flash { > > SSISlave parent_obj; > > > > -uint32_t r; > > - > > Even the trivial cleanup can be a separate patch. > > > BlockBackend *blk; > > > > uint8_t *storage; > > @@ -260,6 +273,9 @@ typedef struct Flash { > > uint8_t cmd_in_progress; > > uint64_t cur_addr; > > bool write_enable; > > +bool four_bytes_address_mode; > > +bool reset_enable; > > +uint8_t extended_addr_reg; > > The datasheets abbreviate this to "EAR" so I think the same should be done > in code. > > > > > int64_t dirty_page; > > > > @@ -397,9 +413,17 @@ void flash_write8(Flash *s, uint64_t addr, > > uint8_t data) > > > > static void complete_collecting_data(Flash *s) { > > -s->cur_addr = s->data[0] << 16; > > -s->cur_addr |= s->data[1] << 8; > > -s->cur_addr |= s->data[2]; > > +if (s->four_bytes_address_mode) { > > +s->cur_addr = s->data[0] << 24; > > +s->cur_addr |= s->data[1] << 16; > > +s->cur_addr |= s->data[2] << 8; > > +
Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35
On 11/26/2015 07:01 PM, Laszlo Ersek wrote: Hello Marcel, [...] if you have ACPI table dumps from within an i440fx SeaBIOS Linux guest, both from before and after your QEMU patches, and those dumps are identical, then that's good evidence against regressions. (I tend to do such acpidump-based comparisons when messing with ACPI builder code.) Hi, OK, there are no functional differences between the SSDT before/after, however the optimization made in patch "1/3 hw/acpi: merge pxb adjacent memory/IO ranges" for pxb-pcies works also for pxb, which is a good thing. SSDT before (only PXB differences) : --- Device (PC0A) { ... Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { ... DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite, 0x, // Granularity 0xFE80, // Range Minimum 0xFE9F, // Range Maximum 0x, // Translation Offset 0x0020, // Length ,, , AddressRangeMemory, TypeStatic) DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite, 0x, // Granularity 0xFE00, // Range Minimum 0xFE7F, // Range Maximum 0x, // Translation Offset 0x0080, // Length ,, , AddressRangeMemory, TypeStatic) ... }) } SSDT after: Device (PC0A) { ... Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { ... DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, NonCacheable, ReadWrite, 0x, // Granularity 0xFE00, // Range Minimum 0xFE9F, // Range Maximum 0x, // Translation Offset 0x00A0, // Length ,, , AddressRangeMemory, TypeStatic) ... }) } As it can be seen, the optimization works also for PXB by merging the MEM regions. Thanks, Marcel [...]
Re: [Qemu-devel] [PULL 07/15] q35: Check propery to determine if iommu is set
On 11/19/2015 03:36 PM, Michael S. Tsirkin wrote: From: Bandan Das The helper function machine_iommu() isn't necesary. We can directly check for the property. Signed-off-by: Bandan Das Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Signed-off-by: Bandan Das --- include/hw/boards.h | 1 - hw/core/machine.c | 5 - hw/pci-host/q35.c | 2 +- 3 files changed, 1 insertion(+), 7 deletions(-) diff --git a/include/hw/boards.h b/include/hw/boards.h index 3e9a92c..24eb6f0 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -33,7 +33,6 @@ MachineClass *find_default_machine(void); extern MachineState *current_machine; bool machine_usb(MachineState *machine); -bool machine_iommu(MachineState *machine); bool machine_kernel_irqchip_allowed(MachineState *machine); bool machine_kernel_irqchip_required(MachineState *machine); int machine_kvm_shadow_mem(MachineState *machine); diff --git a/hw/core/machine.c b/hw/core/machine.c index f4db340..acca00d 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -462,11 +462,6 @@ bool machine_usb(MachineState *machine) return machine->usb; } -bool machine_iommu(MachineState *machine) -{ -return machine->iommu; -} - bool machine_kernel_irqchip_allowed(MachineState *machine) { return machine->kernel_irqchip_allowed; diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c index c81507d..1fb4707 100644 --- a/hw/pci-host/q35.c +++ b/hw/pci-host/q35.c @@ -506,7 +506,7 @@ static void mch_realize(PCIDevice *d, Error **errp) PAM_EXPAN_BASE + i * PAM_EXPAN_SIZE, PAM_EXPAN_SIZE); } /* Intel IOMMU (VT-d) */ -if (machine_iommu(current_machine)) { +if (object_property_get_bool(qdev_get_machine(), "iommu", NULL)) { Maybe is too late, but this contradicts QEMU usage, as I understand object_property_get_* should be used when we don't know object's type. Why use "iommu" when you can simply call current_machine->iommu ? (if you don't like the wrapper, which is pretty harmless in my opinion) Thanks, Marcel mch_init_dmar(mch); } }
Re: [Qemu-devel] MinGW build
Peter Maydell wrote: > On 27 November 2015 at 19:16, Stefan Weil wrote: >> Yes, that's correct. I just did a short test and replaced "printf" >> by "gnu_printf" in disas/libvixl/utils.h: no more warnings when MinGW >> compiles disas/libvixl/a64/disasm-a64.cc. >> >> I think we can wait for a new version of libvixl which includes the fix, >> no need for a last minute fix for QEMU 2.5. Will you report it to the >> libvixl developers? > > Yes, I've reported it to them, and I agree we don't need to fix > this for 2.5. (There are other warnings with this mingw compiler > anyway.) For my compiler (F23 cross-compiler) and a fairly large configuration (*) this is the only warning. I will test later if this fix for me. Later, Juan. (*): fairly large: Everything that I was able to get libraries from Fedora without having to cross-compile anything by hand.
Re: [Qemu-devel] [PATCH] typedefs: Put them back into alphabetical order
19.11.2015 15:29, Markus Armbruster wrote: > "Please keep this list in alphabetical order" has been more honoured > in the breach than in the observance. Clean up. > > While there, drop a redundant struct declaration. Applied to -trivial, thank you! /mjt
Re: [Qemu-devel] [PATCH] scsi: remove scsi_req_free prototype
Applied to -trivial, thank you! /mjt
Re: [Qemu-devel] [PATCH] Trivial: update comment of struct Object
05.11.2015 10:39, Cao jin wrote: > it don`t has "GSList *interfaces" anymore Andreas, should this be applied? It's been on the list for long already, and you said I shouldn't apply qom patches... Thanks, /mjt > Signed-off-by: Cao jin > --- > include/qom/object.h | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/include/qom/object.h b/include/qom/object.h > index 0bb89d4..e46cc30 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -396,9 +396,6 @@ struct ObjectClass > * As a result, #Object contains a reference to the objects type as its > * first member. This allows identification of the real type of the object > at > * run time. > - * > - * #Object also contains a list of #Interfaces that this object > - * implements. > */ > struct Object > { >
Re: [Qemu-devel] [PATCH] gt64xxx: fix decoding of ISD register
06.11.2015 18:34, Paolo Bonzini wrote: > The GT64xxx's internal registers can be placed above the first 4 GiB > in the address space, but not above the first 64 GiB. Correctly cast > the register to a 64-bit integer, and mask away bits above bit 35. Applied to -trivial, thank you! /mjt
Re: [Qemu-devel] [PATCH v2 1/1] configure: use appropriate code fragment for -fstack-protector checks
12.11.2015 17:04, Rodrigo Rebello wrote: > The check for stack-protector support consisted in compiling and linking > the test program below (output by function write_c_skeleton()) with the > compiler flag -fstack-protector-strong first and then with > -fstack-protector-all if the first one failed to work: ... Applied to -trivial, thank you! /mjt
Re: [Qemu-devel] [PATCH] crypto: avoid two coverity false positive error reports
13.11.2015 20:45, Daniel P. Berrange wrote: > In qcrypto_tls_creds_get_path() coverity complains that > we are checking '*creds' for NULL, despite having > dereferenced it previously. This is harmless bug due > to fact that the trace call was too early. Moving it > after the cleanup gets the desired semantics. ... Applied to -trivial, thank you! /mjt
Re: [Qemu-devel] [PATCH] configure: Diagnose broken linkers directly
24.11.2015 17:55, Peter Maydell wrote: > Currently if the user's compiler works for creating .o files but > their linker is broken such that compiling an executable from a > C file does not work, we will report a misleading error message > about the compiler not supporting __thread (since that happens > to be the first test we run which requires a working linker). > Explicitly check that compile_prog works as well as compile_object, > so that people whose toolchain setup is broken get a more helpful > error message. Applied to -trivial, thanks! /mjt
Re: [Qemu-devel] [PATCH v2] bt: check struct sizes
28.11.2015 18:13, Paolo Bonzini wrote: > On 27/11/2015 18:57, Paolo Bonzini wrote: >> See http://permalink.gmane.org/gmane.linux.bluez.kernel/36505. For >> historical >> reasons these do not use sizeof, and Coverity caught a mistake in >> EVT_ENCRYPT_CHANGE_SIZE. >> >> Note other sizes that seem wrong or inconsistent with the kernel header. >> >> Signed-off-by: Paolo Bonzini > > Hmm, can do better... Can do, or will do, or? Note we've lost last S-o-b (by Markus) as well ;) /mjt
Re: [Qemu-devel] [PATCH for-2.5] bt: avoid unintended sign extension
27.11.2015 15:08, Paolo Bonzini wrote: > In the case of a 4-byte length, shifting a value by 24 may cause > an unintended sign extension when converting from int to size_t. > Use a uint32_t variable instead. Applied to -trivial, thank you! /mjt
Re: [Qemu-devel] [PATCH v2] util/id: fully allocate names table
26.11.2015 00:03, John Snow wrote: > Trivial: this array should be allocated to have ID_MAX entries always. > Otherwise if someone were to forget to expand this table, the assertion > in the id generator won't actually trigger; it will read junk data. Applied to -trivial as a bugfix, with a trivial commit description cleanup, removing this part: > v2: Fix the range assertion, too. Compare against the known actual size > of the table instead of what it "should" be. Thanks! /mjt
Re: [Qemu-devel] [PATCH V3 0/3] hw/pcie: Multi-root support for Q35
On 11/27/2015 07:04 PM, Igor Mammedov wrote: On Thu, 26 Nov 2015 20:35:59 +0200 Marcel Apfelbaum wrote: On 11/26/2015 07:01 PM, Laszlo Ersek wrote: Hello Marcel, On 11/26/15 17:00, Marcel Apfelbaum wrote: Note: I took the liberty to CC all the reviewers that took their time and had a look on the previous version, thanks!! The PXB host bridge provides a way to have multiple PCI hierarchies (PCI root buses). This series introduces the pxb-pcie counterpart for PCI Express machines(Currently Q35). This approach works because the Root Complexes are exposed to guest as regular (legacy) opaque PCI host bridges. Tested on Fedora and Windows guests with both Root Ports and PCIe Switches. v2 -> v3: Addressed Eduardo Habkost comments: - Added a bus property to PC machines and use it when querying bus 0. Addressed comments from multiple reviewers (Paolo,Markus,Gerd,Michael) - The issue was the backport compatibility when the PXB changes. - Following all the comments I chose: - Leave the PXB intact as it does the job and all its features (including the internal pci bridge) makes sense. - Add a new device that re-uses all the PXB code but is exposed as a different device to guests. - Once the functionality of the new device diverges we will have no problem to separate the code. I don't think I can productively contribute to the review of this series, but at least I'll try to follow the comments of others. Hi Laszlo, Thank you for your comments. Also, your first patch looks like it touches code that is shared by the i440fx PXB. I think it should cause no change in observable behavior, right? This was the intention. Yes, it should be no change visible to the guest. Should I regression test it with OVMF? If so, that might take... forever. :( I am planning to do this myself, I might ping you if I can't compile/run it. I also think that the new device will work out of the box with Q35 + OVMF. On the other hand, if you have ACPI table dumps from within an i440fx SeaBIOS Linux guest, both from before and after your QEMU patches, and those dumps are identical, then that's good evidence against regressions. (I tend to do such acpidump-based comparisons when messing with ACPI builder code.) This is a good idea, I am going to compare the dumps and get back to you. To do it, you can use patch from my drop_ASL_support_v1 branch: "tests: acpi: print ASL diff in verbose mode" https://github.com/imammedo/qemu/commit/2df59d77151029554a90ce53c059860babadaf30 Hi Igor, Thank you for the pointer, goot to know we have this, but in my case in order to do it I need to change the QEMU command line, re-create the expected asl file, then the expected binary files, and in the end apply my patches and run the test with "V" environment variable. But it may took less than loading the guest. Thanks! Marcel Thanks (and sorry I can't help more ATM) No problem, thank you for your time! Marcel Laszlo v1 -> v2: Addressed Gerd Hoffmann comments: - Added x-enable-internal-bridge compat property to keep the PCI bridge for older machine to avoid breaking migration. Thanks, Marcel Marcel Apfelbaum (3): hw/acpi: merge pxb adjacent memory/IO ranges hw/pxb: introduce pxb-pcie expander for PCIe machines hw/i386: extend pxb query for all PC machines hw/i386/acpi-build.c| 126 +--- hw/i386/pc.c| 2 +- hw/i386/pc_piix.c | 1 + hw/i386/pc_q35.c| 1 + hw/pci-bridge/pci_expander_bridge.c | 98 +++- include/hw/i386/pc.h| 1 + include/hw/pci/pci.h| 1 + 7 files changed, 163 insertions(+), 67 deletions(-)
Re: [Qemu-devel] RFC: raspberry pi / pi2 / Windows-on-ARM support
On 25/11/2015 19:14, Andrew Baumann wrote: > How does this work out with object_property_add_alias? It can fail > and returns an error, so it should be called from realize, but it > adds a new property on the object, so the property won't be useful if > it can only be set after realize has added it. If you know it cannot fail, just pass &error_abort and call it from instance_init. Paolo
Re: [Qemu-devel] [PATCH V3 3/3] hw/i386: extend pxb query for all PC machines
On 11/27/2015 07:28 PM, Eduardo Habkost wrote: On Thu, Nov 26, 2015 at 06:00:28PM +0200, Marcel Apfelbaum wrote: Add bus property to PC machines and use it when looking for primary PCI root bus (bus 0). Signed-off-by: Marcel Apfelbaum I can't pretend I have reviewed the q35 part, but the changes are an improvement to the existing code that depended on find_i440fx(). Acked-by: Eduardo Habkost Thanks! BTW, what's missing to allow us to change acpi_set_pci_info() to use PCMachine::bus instead of find_i440fx(), too? How much of the PCI hotplug stuff is different in q35? It is pretty different. i440fx has acpi based hotplug while q35 has PCIe native hotplug. Since is "native", no acpi info is necessary. Having said that, if we have an PCIe-PCI bridge, the pci devices behind it cannot be hotplugged/unplugged right now. Once we decide to add hotplug support for this scenario, maybe we can get rid of find_i440fx(). Thanks, Marcel --- hw/i386/acpi-build.c | 3 +-- hw/i386/pc.c | 2 +- hw/i386/pc_piix.c| 1 + hw/i386/pc_q35.c | 1 + include/hw/i386/pc.h | 1 + 5 files changed, 5 insertions(+), 3 deletions(-) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index 736b252..bca3f06 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -950,8 +950,7 @@ build_ssdt(GArray *table_data, GArray *linker, /* Reserve space for header */ acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader)); -/* Extra PCI root buses are implemented only for i440fx */ -bus = find_i440fx(); +bus = PC_MACHINE(machine)->bus; if (bus) { QLIST_FOREACH(bus, &bus->child, sibling) { uint8_t bus_num = pci_bus_num(bus); diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 5e20e07..07697ed 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1174,7 +1174,7 @@ void pc_guest_info_machine_done(Notifier *notifier, void *data) PcGuestInfoState *guest_info_state = container_of(notifier, PcGuestInfoState, machine_done); -PCIBus *bus = find_i440fx(); +PCIBus *bus = PC_MACHINE(qdev_get_machine())->bus; if (bus) { int extra_hosts = 0; diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 07d0baa..ca6ef9a 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -197,6 +197,7 @@ static void pc_init1(MachineState *machine, pcms->below_4g_mem_size, pcms->above_4g_mem_size, pci_memory, ram_memory); +pcms->bus = pci_bus; } else { pci_bus = NULL; i440fx_state = NULL; diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 0fdae09..822b3e5 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -192,6 +192,7 @@ static void pc_q35_init(MachineState *machine) qdev_init_nofail(DEVICE(q35_host)); phb = PCI_HOST_BRIDGE(q35_host); host_bus = phb->bus; +pcms->bus = phb->bus; /* create ISA bus */ lpc = pci_create_simple_multifunction(host_bus, PCI_DEVFN(ICH9_LPC_DEV, ICH9_LPC_FUNC), true, diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 854c330..e42771c 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -41,6 +41,7 @@ struct PCMachineState { OnOffAuto smm; bool enforce_aligned_dimm; ram_addr_t below_4g_mem_size, above_4g_mem_size; +PCIBus *bus; }; #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device" -- 2.1.0
[Qemu-devel] [RFC PATCH 2/2] ARM: Virt: ACPI: Add GIC ITS description in ACPI MADT table
From: Shannon Zhao If GIC ITS is supported, add description in ACPI MADT table, then guest could use ITS when booting with ACPI. Signed-off-by: Shannon Zhao --- hw/arm/virt-acpi-build.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 3c2c5d6..f200086 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -41,6 +41,7 @@ #include "hw/acpi/aml-build.h" #include "hw/pci/pcie_host.h" #include "hw/pci/pci.h" +#include "kvm_arm.h" #define ARM_SPI_BASE 32 @@ -440,6 +441,7 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info, AcpiMultipleApicTable *madt; AcpiMadtGenericDistributor *gicd; AcpiMadtGenericMsiFrame *gic_msi; +AcpiMadtGenericTranslator *gic_its; int i; madt = acpi_data_push(table_data, sizeof *madt); @@ -475,6 +477,15 @@ build_madt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info, gicr->length = sizeof(*gicr); gicr->base_address = cpu_to_le64(memmap[VIRT_GIC_REDIST].base); gicr->range_length = cpu_to_le32(memmap[VIRT_GIC_REDIST].size); + +if (!its_class_name()) { +return; +} +gic_its = acpi_data_push(table_data, sizeof *gic_its); +gic_its->type = ACPI_APIC_GENERIC_TRANSLATOR; +gic_its->length = sizeof(*gic_its); +gic_its->translation_id = 0; +gic_its->base_address = cpu_to_le64(memmap[VIRT_GIC_ITS].base); } else { gic_msi = acpi_data_push(table_data, sizeof *gic_msi); gic_msi->type = ACPI_APIC_GENERIC_MSI_FRAME; -- 2.0.4
[Qemu-devel] [RFC PATCH 0/2] Add GIC ITS description in ACPI MADT table
From: Shannon Zhao These two patches add ITS description in ACPI MADT table. It bases on Pavel Fedin's ITS series[1]. [1]https://www.mail-archive.com/qemu-devel@nongnu.org/msg337421.html Shannon Zhao (2): ACPI: Add GIC Interrupt Translation Service Structure definition ARM: Virt: ACPI: Add GIC ITS description in ACPI MADT table hw/arm/virt-acpi-build.c| 11 +++ include/hw/acpi/acpi-defs.h | 13 - 2 files changed, 23 insertions(+), 1 deletion(-) -- 2.0.4
[Qemu-devel] [RFC PATCH 1/2] ACPI: Add GIC Interrupt Translation Service Structure definition
From: Shannon Zhao ACPI Spec 6.0 introduces GIC Interrupt Translation Service Structure. Here we add the definition of the Structure. Signed-off-by: Shannon Zhao --- include/hw/acpi/acpi-defs.h | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h index c7a03d4..fc273ce 100644 --- a/include/hw/acpi/acpi-defs.h +++ b/include/hw/acpi/acpi-defs.h @@ -294,7 +294,8 @@ typedef struct AcpiMultipleApicTable AcpiMultipleApicTable; #define ACPI_APIC_GENERIC_DISTRIBUTOR 12 #define ACPI_APIC_GENERIC_MSI_FRAME 13 #define ACPI_APIC_GENERIC_REDISTRIBUTOR 14 -#define ACPI_APIC_RESERVED 15 /* 15 and greater are reserved */ +#define ACPI_APIC_GENERIC_TRANSLATOR15 +#define ACPI_APIC_RESERVED 16 /* 16 and greater are reserved */ /* * MADT sub-structures (Follow MULTIPLE_APIC_DESCRIPTION_TABLE) @@ -393,6 +394,16 @@ struct AcpiMadtGenericRedistributor { typedef struct AcpiMadtGenericRedistributor AcpiMadtGenericRedistributor; +struct AcpiMadtGenericTranslator { +ACPI_SUB_HEADER_DEF +uint16_t reserved; +uint32_t translation_id; +uint64_t base_address; +uint32_t reserved2; +} QEMU_PACKED; + +typedef struct AcpiMadtGenericTranslator AcpiMadtGenericTranslator; + /* * Generic Timer Description Table (GTDT) */ -- 2.0.4
Re: [Qemu-devel] [RFC PATCH v3 1/5] hw/intc: Implement ITS base class
On 2015/11/24 18:13, Pavel Fedin wrote: > +static const VMStateDescription vmstate_its = { > +.name = "arm_gicv3_its", > +.pre_save = gicv3_its_pre_save, > +.post_load = gicv3_its_post_load, > +.fields = (VMStateField[]) { > +VMSTATE_UINT32(ctlr, GICv3ITSState), > +VMSTATE_UINT64(cbaser, GICv3ITSState), > +VMSTATE_UINT64(cwriter, GICv3ITSState), > +VMSTATE_UINT64(creadr, GICv3ITSState), > +VMSTATE_UINT64_ARRAY(baser, GICv3ITSState, 8), > +VMSTATE_END_OF_LIST() > +} lack a comma here. > +.unmigratable = true, > +}; hw/intc/arm_gicv3_its_common.c:57:5: error: request for member ‘unmigratable’ in something not a structure or union .unmigratable = true, ^ -- Shannon