Re: [Qemu-devel] [PATCH v11 09/14] target-mips: Add ASE DSP bit/manipulation instructions
On Wed, Oct 17, 2012 at 11:44:41AM +0800, Jia Liu wrote: Hi Aurelien, +#ifdef TARGET_MIPS64 +case OPC_ABSQ_S_QH_DSP: +switch (op2) { +case OPC_REPL_OB: +check_dsp(ctx); +{ +target_long temp; + +imm = (ctx-opcode 16) 0xFF; +temp = imm; +temp = (temp 8) | temp; +temp = (temp 16) | temp; +temp = (temp 32) | temp; +tcg_gen_movi_tl(cpu_gpr[ret], temp); +break; +} This hasn't been fixed, and thus is still wrong. Thank you for check this again. May you give me more comment about this please? I'm not sure what should do here. The instruction is defined as: | Replicate a immediate byte into all elements of an eight byte vector. | | Description: rd ← immediate || immediate || immediate || immediate || immediate || | immediate || immediate || immediate In your code, I only see the value replicated 4 times, not 8 times. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH v11 06/14] target-mips: Add ASE DSP arithmetic instructions
On Wed, Oct 17, 2012 at 12:54:45PM +0800, Jia Liu wrote: +if (ret == 0) { +/* Treat as NOP. */ +MIPS_DEBUG(NOP); +return; +} + +if (v1 == 0) { +tcg_gen_movi_tl(v1_t, 0); +} else { +gen_load_gpr(v1_t, v1); +} + +if (v2 == 0) { +tcg_gen_movi_tl(v2_t, 0); +} else { +gen_load_gpr(v2_t, v2); +} You don't need to check if v1 or v2 == 0, this is already done in gen_load_gpr(). Sorry I'm a little confused here, just remove the check code here is OK? Yes, gen_load_gpr() is defined as: | static inline void gen_load_gpr (TCGv t, int reg) | { | if (reg == 0) | tcg_gen_movi_tl(t, 0); | else | tcg_gen_mov_tl(t, cpu_gpr[reg]); | } so the test for reg == 0 is redundant. Is this code OK? if (ret == 0) { /* Treat as NOP. */ MIPS_DEBUG(NOP); return; } v1_t = tcg_temp_new(); v2_t = tcg_temp_new(); gen_load_gpr(v1_t, v1); gen_load_gpr(v2_t, v2); Yes, it looks fine to me. + +switch (op1) { +/* OPC_MULT_G_2E is equal OPC_ADDUH_QB_DSP */ +case OPC_MULT_G_2E: +check_dspr2(ctx); +switch (op2) { +case OPC_ADDUH_QB: +gen_helper_adduh_qb(cpu_gpr[ret], v1_t, v2_t); +break; +case OPC_ADDUH_R_QB: +gen_helper_adduh_r_qb(cpu_gpr[ret], v1_t, v2_t); +break; +case OPC_ADDQH_PH: +gen_helper_addqh_ph(cpu_gpr[ret], v1_t, v2_t); +break; +case OPC_ADDQH_R_PH: +gen_helper_addqh_r_ph(cpu_gpr[ret], v1_t, v2_t); +break; +case OPC_ADDQH_W: +gen_helper_addqh_w(cpu_gpr[ret], v1_t, v2_t); +break; +case OPC_ADDQH_R_W: +gen_helper_addqh_r_w(cpu_gpr[ret], v1_t, v2_t); +break; +case OPC_SUBUH_QB: +gen_helper_subuh_qb(cpu_gpr[ret], v1_t, v2_t); +break; +case OPC_SUBUH_R_QB: +gen_helper_subuh_r_qb(cpu_gpr[ret], v1_t, v2_t); +break; +case OPC_SUBQH_PH: +gen_helper_subqh_ph(cpu_gpr[ret], v1_t, v2_t); +break; +case OPC_SUBQH_R_PH: +gen_helper_subqh_r_ph(cpu_gpr[ret], v1_t, v2_t); +break; +case OPC_SUBQH_W: +gen_helper_subqh_w(cpu_gpr[ret], v1_t, v2_t); +break; +case OPC_SUBQH_R_W: +gen_helper_subqh_r_w(cpu_gpr[ret], v1_t, v2_t); +break; +} +break; +case OPC_ABSQ_S_PH_DSP: +switch (op2) { +case OPC_ABSQ_S_QB: +check_dspr2(ctx); +gen_helper_absq_s_qb(cpu_gpr[ret], v2_t, cpu_env); +break; +case OPC_ABSQ_S_PH: +check_dsp(ctx); +gen_helper_absq_s_ph(cpu_gpr[ret], v2_t, cpu_env); +break; +case OPC_ABSQ_S_W: +check_dsp(ctx); +gen_helper_absq_s_w(cpu_gpr[ret], v2_t, cpu_env); +break; +case OPC_PRECEQ_W_PHL: +{ +TCGv temp = tcg_temp_new();; +check_dsp(ctx); +tcg_gen_andi_tl(temp, v2_t, 0x); +tcg_gen_ext32s_tl(cpu_gpr[ret], temp); Small optimization; you don't need the temp here, andi can directly write to cpu_gpr[ret]. +tcg_temp_free(temp); +break; +} +case OPC_PRECEQ_W_PHR: +{ +TCGv temp = tcg_temp_new();; +check_dsp(ctx); +tcg_gen_andi_tl(temp, v2_t, 0x); This is not needed due to the shift and the ext below. +tcg_gen_shli_tl(temp, temp, 16); +tcg_gen_ext32s_tl(cpu_gpr[ret], temp); +tcg_temp_free(temp); +break; +} +case OPC_PRECEQU_PH_QBL: +check_dsp(ctx); +gen_helper_precequ_ph_qbl(cpu_gpr[ret], v2_t); +break; +case OPC_PRECEQU_PH_QBR: +check_dsp(ctx); +gen_helper_precequ_ph_qbr(cpu_gpr[ret], v2_t); +break; +case OPC_PRECEQU_PH_QBLA: +check_dsp(ctx); +gen_helper_precequ_ph_qbla(cpu_gpr[ret], v2_t); +break; +case OPC_PRECEQU_PH_QBRA: +check_dsp(ctx); +gen_helper_precequ_ph_qbra(cpu_gpr[ret], v2_t); +break; +case OPC_PRECEU_PH_QBL: +check_dsp(ctx); +gen_helper_preceu_ph_qbl(cpu_gpr[ret], v2_t); +break; +case OPC_PRECEU_PH_QBR: +check_dsp(ctx); +gen_helper_preceu_ph_qbr(cpu_gpr[ret], v2_t); +break; +case
Re: [Qemu-devel] [PATCH 1/2] hmp: fix info cpus for sparc targets
On Tue, Oct 16, 2012 at 11:01:45PM -0300, Luiz Capitulino wrote: On Wed, 17 Oct 2012 01:28:34 +0200 Aurelien Jarno aurel...@aurel32.net wrote: On sparc targets, info cpus returns this kind of output: | info cpus | * CPU #0: pc=0x00424d18pc=0x00424d18npc=0x00424d1c thread_id=19460 pc is printed twice, there is no space between pc, pc and npc. As npc implies pc, don't set has_pc on sparc, and add a space between pc and npc. Cc: Luiz Capitulino lcapitul...@redhat.com Cc: Markus Armbruster arm...@redhat.com Cc: Blue Swirl blauwir...@gmail.com Signed-off-by: Aurelien Jarno aurel...@aurel32.net --- cpus.c |2 -- hmp.c |2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/cpus.c b/cpus.c index 750a76f..cbc8556 100644 --- a/cpus.c +++ b/cpus.c @@ -1216,8 +1216,6 @@ CpuInfoList *qmp_query_cpus(Error **errp) info-value-has_nip = true; info-value-nip = env-nip; #elif defined(TARGET_SPARC) -info-value-has_pc = true; -info-value-pc = env-pc; info-value-has_npc = true; info-value-npc = env-npc; #elif defined(TARGET_MIPS) diff --git a/hmp.c b/hmp.c index 70bdec2..64b6ab5 100644 --- a/hmp.c +++ b/hmp.c @@ -242,7 +242,7 @@ void hmp_info_cpus(Monitor *mon) monitor_printf(mon, nip=0x%016 PRIx64, cpu-value-nip); } if (cpu-value-has_npc) { -monitor_printf(mon, pc=0x%016 PRIx64, cpu-value-pc); +monitor_printf(mon, pc=0x%016 PRIx64, cpu-value-pc); This patch changes qmp_query_cpus() to never set pc, so why do you print it? Well the print part of the code isn't change beside adding a space at the end. With the current code, has_npc implies has_pc, that's why I have removed the latter from qmp_query_cpus() in the sparc case. If you prefer to change this behavior and not have has_npc implying has_pc, I can send another patch. monitor_printf(mon, npc=0x%016 PRIx64, cpu-value-npc); } if (cpu-value-has_PC) { -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH v2 0/5] target-mips: Preparations for CPUState part 4b series
On Fri, Oct 12, 2012 at 12:56:32AM +0200, Andreas Färber wrote: Hello Aurélien, This series picks up some preparatory patches for QOM CPUState refactoring, originally posted in May. They still applied cleanly, but I optimized them a bit and expanded the explanations. In short it is about MIPSCPU vs. CPUMIPSState; more fields will be moved from CPU_COMMON macro to CPUState struct, and to access CPUState we need a MIPSCPU. Thus MIPSCPU is preferable for arguments of static helpers (not TCG helpers) because we save some redundant accessor/cast macros. Can you please ack/apply and keep in mind for the current patch review? Available for testing from: git://github.com/afaerber/qemu-cpu.git qom-cpu-mips https://github.com/afaerber/qemu-cpu/commits/qom-cpu-mips Regards, Andreas Cc: Aurélien Jarno aurel...@aurel32.net Cc: Jia Liu pro...@gmail.com Cc: Eric Johnson er...@mips.com Cc: Richard Henderson r...@twiddle.net v1 - v2: * Cherry-picked from my CPUState part 4 series * Avoided calling mips_env_get_cpu() in both if branches of helper_m[t]tc0_tchalt() * Prepended patch to clean up resulting variable naming mess * Placed variable declarations in the closest block (requested by Alex elsewhere) Andreas Färber (5): target-mips: Clean up other_cpu in helper_{d,e}vpe() target-mips: Pass MIPSCPU to mips_tc_wake() target-mips: Pass MIPSCPU to mips_vpe_is_wfi() target-mips: Pass MIPSCPU to mips_tc_sleep() target-mips: Pass MIPSCPU to mips_vpe_sleep() target-mips/op_helper.c | 63 +-- 1 Datei geändert, 39 Zeilen hinzugefügt(+), 24 Zeilen entfernt(-) -- 1.7.10.4 Thanks, all applied. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net
Re: [Qemu-devel] [PATCH v11 09/14] target-mips: Add ASE DSP bit/manipulation instructions
Hi Aurelien, On Wed, Oct 17, 2012 at 2:05 PM, Aurelien Jarno aurel...@aurel32.net wrote: On Wed, Oct 17, 2012 at 11:44:41AM +0800, Jia Liu wrote: Hi Aurelien, +#ifdef TARGET_MIPS64 +case OPC_ABSQ_S_QH_DSP: +switch (op2) { +case OPC_REPL_OB: +check_dsp(ctx); +{ +target_long temp; + +imm = (ctx-opcode 16) 0xFF; +temp = imm; +temp = (temp 8) | temp; +temp = (temp 16) | temp; +temp = (temp 32) | temp; +tcg_gen_movi_tl(cpu_gpr[ret], temp); +break; +} This hasn't been fixed, and thus is still wrong. Thank you for check this again. May you give me more comment about this please? I'm not sure what should do here. The instruction is defined as: | Replicate a immediate byte into all elements of an eight byte vector. | | Description: rd ← immediate || immediate || immediate || immediate || immediate || | immediate || immediate || immediate In your code, I only see the value replicated 4 times, not 8 times. Thank you very much, is this code OK? case OPC_REPL_OB: check_dsp(ctx); { target_long temp; imm = (ctx-opcode 16) 0xFF; temp = ((uint64_t)imm 56) | ((uint64_t)imm 48) | ((uint64_t)imm 40) | ((uint64_t)imm 32) | ((uint64_t)imm 24) | ((uint64_t)imm 16) | ((uint64_t)imm 8) | (uint64_t)imm; tcg_gen_movi_tl(cpu_gpr[ret], temp); break; } -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net Regards, Jia.
[Qemu-devel] [PATCH 4/7] serial: add 2x + 4x pci variant
Add multiport serial card implementation, with two variants, one featuring two and one featuring four ports. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/serial-pci.c | 149 +++ 1 files changed, 149 insertions(+), 0 deletions(-) diff --git a/hw/serial-pci.c b/hw/serial-pci.c index 6fcf117..badd297 100644 --- a/hw/serial-pci.c +++ b/hw/serial-pci.c @@ -26,11 +26,23 @@ #include serial.h #include pci.h +#define PCI_SERIAL_MAX_PORTS 4 + typedef struct PCISerialState { PCIDevice dev; SerialState state; } PCISerialState; +typedef struct PCIMultiSerialState { +PCIDevicedev; +MemoryRegion iobar; +uint32_t ports; +char *name[PCI_SERIAL_MAX_PORTS]; +SerialState state[PCI_SERIAL_MAX_PORTS]; +uint32_t level[PCI_SERIAL_MAX_PORTS]; +qemu_irq *irqs; +} PCIMultiSerialState; + static int serial_pci_init(PCIDevice *dev) { PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev); @@ -47,6 +59,56 @@ static int serial_pci_init(PCIDevice *dev) return 0; } +static void multi_serial_irq_mux(void *opaque, int n, int level) +{ +PCIMultiSerialState *pci = opaque; +int i, pending = 0; + +pci-level[n] = level; +for (i = 0; i pci-ports; i++) { +if (pci-level[i]) { +pending = 1; +} +} +qemu_set_irq(pci-dev.irq[0], pending); +} + +static int multi_serial_pci_init(PCIDevice *dev) +{ +PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev); +PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev); +SerialState *s; +int i; + +switch (pc-device_id) { +case 0x0003: +pci-ports = 2; +break; +case 0x0004: +pci-ports = 4; +break; +} +assert(pci-ports 0); +assert(pci-ports = PCI_SERIAL_MAX_PORTS); + +pci-dev.config[PCI_INTERRUPT_PIN] = 0x01; +memory_region_init(pci-iobar, multiserial, 8 * pci-ports); +pci_register_bar(pci-dev, 0, PCI_BASE_ADDRESS_SPACE_IO, pci-iobar); +pci-irqs = qemu_allocate_irqs(multi_serial_irq_mux, pci, + pci-ports); + +for (i = 0; i pci-ports; i++) { +s = pci-state + i; +s-baudbase = 115200; +serial_init_core(s); +s-irq = pci-irqs[i]; +pci-name[i] = g_strdup_printf(uart #%d, i+1); +memory_region_init_io(s-io, serial_io_ops, s, pci-name[i], 8); +memory_region_add_subregion(pci-iobar, 8 * i, s-io); +} +return 0; +} + static void serial_pci_exit(PCIDevice *dev) { PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev); @@ -56,6 +118,22 @@ static void serial_pci_exit(PCIDevice *dev) memory_region_destroy(s-io); } +static void multi_serial_pci_exit(PCIDevice *dev) +{ +PCIMultiSerialState *pci = DO_UPCAST(PCIMultiSerialState, dev, dev); +SerialState *s; +int i; + +for (i = 0; i pci-ports; i++) { +s = pci-state + i; +serial_exit_core(s); +memory_region_destroy(s-io); +g_free(pci-name[i]); +} +memory_region_destroy(pci-iobar); +qemu_free_irqs(pci-irqs); +} + static const VMStateDescription vmstate_pci_serial = { .name = pci-serial, .version_id = 1, @@ -67,11 +145,38 @@ static const VMStateDescription vmstate_pci_serial = { } }; +static const VMStateDescription vmstate_pci_multi_serial = { +.name = pci-serial-multi, +.version_id = 1, +.minimum_version_id = 1, +.fields = (VMStateField[]) { +VMSTATE_PCI_DEVICE(dev, PCIMultiSerialState), +VMSTATE_STRUCT_ARRAY(state, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS, + 0, vmstate_serial, SerialState), +VMSTATE_UINT32_ARRAY(level, PCIMultiSerialState, PCI_SERIAL_MAX_PORTS), +VMSTATE_END_OF_LIST() +} +}; + static Property serial_pci_properties[] = { DEFINE_PROP_CHR(chardev, PCISerialState, state.chr), DEFINE_PROP_END_OF_LIST(), }; +static Property multi_2x_serial_pci_properties[] = { +DEFINE_PROP_CHR(chardev1, PCIMultiSerialState, state[0].chr), +DEFINE_PROP_CHR(chardev2, PCIMultiSerialState, state[1].chr), +DEFINE_PROP_END_OF_LIST(), +}; + +static Property multi_4x_serial_pci_properties[] = { +DEFINE_PROP_CHR(chardev1, PCIMultiSerialState, state[0].chr), +DEFINE_PROP_CHR(chardev2, PCIMultiSerialState, state[1].chr), +DEFINE_PROP_CHR(chardev3, PCIMultiSerialState, state[2].chr), +DEFINE_PROP_CHR(chardev4, PCIMultiSerialState, state[3].chr), +DEFINE_PROP_END_OF_LIST(), +}; + static void serial_pci_class_initfn(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -86,6 +191,34 @@ static void serial_pci_class_initfn(ObjectClass *klass, void *data) dc-props = serial_pci_properties; } +static void multi_2x_serial_pci_class_initfn(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +PCIDeviceClass *pc =
[Qemu-devel] [PATCH 6/7] usb-serial: don't magically zap chardev on umplug
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/usb/dev-serial.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c index 69b6e48..43214cd 100644 --- a/hw/usb/dev-serial.c +++ b/hw/usb/dev-serial.c @@ -421,7 +421,7 @@ static void usb_serial_handle_destroy(USBDevice *dev) { USBSerialState *s = (USBSerialState *)dev; -qemu_chr_delete(s-cs); +qemu_chr_add_handlers(s-cs, NULL, NULL, NULL, NULL); } static int usb_serial_can_read(void *opaque) -- 1.7.1
[Qemu-devel] [PULL v4 0/7] serial device hotplug patch series.
Hi, Next round of the serial device hotplug series. Left out chardev patches due to confligting in-flight patches and ongoing discussions, carrying only pci-serial and usb-serial bits. Adressed Anthonys comments: Added license to serial.h and moved documentation to docs/specs/pci-serial.txt. please pull, Gerd The following changes since commit 6f4d6b09088ee161ff4be0e4db4e4c0962c79070: target-mips: Pass MIPSCPU to mips_vpe_sleep() (2012-10-17 01:32:11 +0200) are available in the git repository at: git://git.kraxel.org/qemu serial.3 Gerd Hoffmann (7): serial: split serial.c serial: add pci variant serial: add windows inf file for the pci card to docs serial: add 2x + 4x pci variant serial: add pci-serial documentation usb-serial: don't magically zap chardev on umplug usb-serial: only expose device in guest when the chardev is open default-configs/pci.mak |2 + docs/qemupciserial.inf| 109 +++ docs/specs/pci-serial.txt | 34 ++ hw/Makefile.objs |3 +- hw/alpha_dp264.c |1 + hw/kzm.c |2 +- hw/mips_fulong2e.c|1 + hw/mips_jazz.c|1 + hw/mips_malta.c |1 + hw/mips_mipssim.c |2 +- hw/mips_r4k.c |1 + hw/musicpal.c |2 +- hw/omap_uart.c|3 +- hw/openrisc_sim.c |3 +- hw/pc.c |1 + hw/pc.h | 27 - hw/pci_ids.h |1 + hw/petalogix_ml605_mmu.c |2 +- hw/ppc/e500.c |2 +- hw/ppc405_uc.c|2 +- hw/ppc440_bamboo.c|2 +- hw/ppc_prep.c |1 + hw/pxa2xx.c |2 +- hw/serial-isa.c | 130 +++ hw/serial-pci.c | 252 + hw/serial.c | 149 ++- hw/serial.h | 99 ++ hw/sm501.c|2 +- hw/sun4u.c|1 + hw/usb/dev-serial.c | 21 +++- hw/virtex_ml507.c |2 +- hw/xtensa_lx60.c |3 +- 32 files changed, 681 insertions(+), 183 deletions(-) create mode 100644 docs/qemupciserial.inf create mode 100644 docs/specs/pci-serial.txt create mode 100644 hw/serial-isa.c create mode 100644 hw/serial-pci.c create mode 100644 hw/serial.h
[Qemu-devel] [PATCH 3/7] serial: add windows inf file for the pci card to docs
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- docs/qemupciserial.inf | 109 1 files changed, 109 insertions(+), 0 deletions(-) create mode 100644 docs/qemupciserial.inf diff --git a/docs/qemupciserial.inf b/docs/qemupciserial.inf new file mode 100644 index 000..3474310 --- /dev/null +++ b/docs/qemupciserial.inf @@ -0,0 +1,109 @@ +; qemupciserial.inf for QEMU, based on MSPORTS.INF + +; The driver itself is shipped with Windows (serial.sys). This is +; just a inf file to tell windows which pci id the serial pci card +; emulated by qemu has, and to apply a name tag to it which windows +; will show in the device manager. + +; Installing the driver: Go to device manager. You should find a pci +; serial card tagged with a yellow question mark. Open properties. +; Pick update driver. Then select driver manually. Pick Ports +; (Com+Lpt) from the list. Click Have a disk. Select this file. +; Procedure may vary a bit depending on the windows version. + +; FIXME: This file covers the single port version only. + +[Version] +Signature=$CHICAGO$ +Class=Ports +ClassGuid={4D36E978-E325-11CE-BFC1-08002BE10318} +Provider=%QEMU% +DriverVer=09/24/2012,1.3.0 + +[SourceDisksNames] +3426=windows cd + +[SourceDisksFiles] +serial.sys = 3426 +serenum.sys= 3426 + +[DestinationDirs] +DefaultDestDir = 11;LDID_SYS +ComPort.NT.Copy = 12;DIRID_DRIVERS +SerialEnumerator.NT.Copy=12 ;DIRID_DRIVERS + +; Drivers +;-- +[Manufacturer] +%QEMU%=QEMU,NTx86 + +[QEMU.NTx86] +%QEMU-PCI_SERIAL.DeviceDesc% = ComPort, PCI\VEN_1b36DEV_0002CC_0700 + +; COM sections +;-- +[ComPort.AddReg] +HKR,,PortSubClass,1,01 + +[ComPort.NT] +AddReg=ComPort.AddReg, ComPort.NT.AddReg +LogConfig=caa +SyssetupPnPFlags = 1 + +[ComPort.NT.HW] +AddReg=ComPort.NT.HW.AddReg + +[ComPort.NT.AddReg] +HKR,,EnumPropPages32,,MsPorts.dll,SerialPortPropPageProvider + +[ComPort.NT.HW.AddReg] +HKR,,UpperFilters,0x0001,serenum + +;-- Service installation +; Port Driver (function driver for this device) +[ComPort.NT.Services] +AddService = Serial, 0x0002, Serial_Service_Inst, Serial_EventLog_Inst +AddService = Serenum,,Serenum_Service_Inst + +; -- Serial Port Driver install sections +[Serial_Service_Inst] +DisplayName= %Serial.SVCDESC% +ServiceType= 1 ; SERVICE_KERNEL_DRIVER +StartType = 1 ; SERVICE_SYSTEM_START (this driver may do detection) +ErrorControl = 0 ; SERVICE_ERROR_IGNORE +ServiceBinary = %12%\serial.sys +LoadOrderGroup = Extended base + +; -- Serenum Driver install section +[Serenum_Service_Inst] +DisplayName= %Serenum.SVCDESC% +ServiceType= 1 ; SERVICE_KERNEL_DRIVER +StartType = 3 ; SERVICE_DEMAND_START +ErrorControl = 1 ; SERVICE_ERROR_NORMAL +ServiceBinary = %12%\serenum.sys +LoadOrderGroup = PNP Filter + +[Serial_EventLog_Inst] +AddReg = Serial_EventLog_AddReg + +[Serial_EventLog_AddReg] +HKR,,EventMessageFile,0x0002,%%SystemRoot%%\System32\IoLogMsg.dll;%%SystemRoot%%\System32\drivers\serial.sys +HKR,,TypesSupported,0x00010001,7 + +; The following sections are COM port resource configs. +; Section name format means: +; Char 1 = c (COM port) +; Char 2 = I/O config: 1 (3f8), 2 (2f8), 3 (3e8), 4 (2e8), a (any) +; Char 3 = IRQ config: #, a (any) + +[caa] ; Any base, any IRQ +ConfigPriority=HARDRECONFIG +IOConfig=8@100-%fff8(3ff::) +IRQConfig=S:3,4,5,7,9,10,11,12,14,15 + +[Strings] +QEMU=QEMU +QEMU-PCI_SERIAL.DeviceDesc=QEMU Serial PCI Card + +Serial.SVCDESC = Serial port driver +Serenum.SVCDESC = Serenum Filter Driver -- 1.7.1
[Qemu-devel] [PATCH 1/7] serial: split serial.c
Split serial.c into serial.c, serial.h and serial-isa.c. While being at creating a serial.h header file move the serial prototypes from pc.h to the new serial.h. The latter leads to s/pc.h/serial.h/ in tons of boards which just want the serial bits from pc.h Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/Makefile.objs |2 +- hw/alpha_dp264.c |1 + hw/kzm.c |2 +- hw/mips_fulong2e.c |1 + hw/mips_jazz.c |1 + hw/mips_malta.c |1 + hw/mips_mipssim.c|2 +- hw/mips_r4k.c|1 + hw/musicpal.c|2 +- hw/omap_uart.c |3 +- hw/openrisc_sim.c|3 +- hw/pc.c |1 + hw/pc.h | 27 - hw/petalogix_ml605_mmu.c |2 +- hw/ppc/e500.c|2 +- hw/ppc405_uc.c |2 +- hw/ppc440_bamboo.c |2 +- hw/ppc_prep.c|1 + hw/pxa2xx.c |2 +- hw/serial-isa.c | 130 + hw/serial.c | 143 ++ hw/serial.h | 98 +++ hw/sm501.c |2 +- hw/sun4u.c |1 + hw/virtex_ml507.c|2 +- hw/xtensa_lx60.c |3 +- 26 files changed, 257 insertions(+), 180 deletions(-) create mode 100644 hw/serial-isa.c create mode 100644 hw/serial.h diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 854faa9..16e7a1e 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -20,7 +20,7 @@ common-obj-$(CONFIG_M48T59) += m48t59.o common-obj-$(CONFIG_ESCC) += escc.o common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o -common-obj-$(CONFIG_SERIAL) += serial.o +common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o common-obj-$(CONFIG_PARALLEL) += parallel.o common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o common-obj-$(CONFIG_PCSPK) += pcspk.o diff --git a/hw/alpha_dp264.c b/hw/alpha_dp264.c index 5ea04c7..8ce04e5 100644 --- a/hw/alpha_dp264.c +++ b/hw/alpha_dp264.c @@ -15,6 +15,7 @@ #include mc146818rtc.h #include ide.h #include i8254.h +#include serial.h #define MAX_IDE_BUS 2 diff --git a/hw/kzm.c b/hw/kzm.c index 68cd1b4..1f3082b 100644 --- a/hw/kzm.c +++ b/hw/kzm.c @@ -21,7 +21,7 @@ #include net.h #include sysemu.h #include boards.h -#include pc.h /* for the FPGA UART that emulates a 16550 */ +#include serial.h #include imx.h /* Memory map for Kzm Emulation Baseboard: diff --git a/hw/mips_fulong2e.c b/hw/mips_fulong2e.c index d4a8672..a3cb3ab 100644 --- a/hw/mips_fulong2e.c +++ b/hw/mips_fulong2e.c @@ -20,6 +20,7 @@ #include hw.h #include pc.h +#include serial.h #include fdc.h #include net.h #include boards.h diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c index db927f1..d35cd54 100644 --- a/hw/mips_jazz.c +++ b/hw/mips_jazz.c @@ -26,6 +26,7 @@ #include mips.h #include mips_cpudevs.h #include pc.h +#include serial.h #include isa.h #include fdc.h #include sysemu.h diff --git a/hw/mips_malta.c b/hw/mips_malta.c index 632b466..8f73b1b 100644 --- a/hw/mips_malta.c +++ b/hw/mips_malta.c @@ -24,6 +24,7 @@ #include hw.h #include pc.h +#include serial.h #include fdc.h #include net.h #include boards.h diff --git a/hw/mips_mipssim.c b/hw/mips_mipssim.c index 830f635..0ee6756 100644 --- a/hw/mips_mipssim.c +++ b/hw/mips_mipssim.c @@ -27,7 +27,7 @@ #include hw.h #include mips.h #include mips_cpudevs.h -#include pc.h +#include serial.h #include isa.h #include net.h #include sysemu.h diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c index 967a76e..b3be80b 100644 --- a/hw/mips_r4k.c +++ b/hw/mips_r4k.c @@ -11,6 +11,7 @@ #include mips.h #include mips_cpudevs.h #include pc.h +#include serial.h #include isa.h #include net.h #include sysemu.h diff --git a/hw/musicpal.c b/hw/musicpal.c index f305e21..346fe41 100644 --- a/hw/musicpal.c +++ b/hw/musicpal.c @@ -15,7 +15,7 @@ #include net.h #include sysemu.h #include boards.h -#include pc.h +#include serial.h #include qemu-timer.h #include ptimer.h #include block.h diff --git a/hw/omap_uart.c b/hw/omap_uart.c index 167d5c4..1c16a54 100644 --- a/hw/omap_uart.c +++ b/hw/omap_uart.c @@ -20,8 +20,7 @@ #include qemu-char.h #include hw.h #include omap.h -/* We use pc-style serial ports. */ -#include pc.h +#include serial.h #include exec-memory.h /* UARTs */ diff --git a/hw/openrisc_sim.c b/hw/openrisc_sim.c index 55e97f0..e484613 100644 --- a/hw/openrisc_sim.c +++ b/hw/openrisc_sim.c @@ -21,7 +21,8 @@ #include hw.h #include boards.h #include elf.h -#include pc.h +#include serial.h +#include net.h #include loader.h #include exec-memory.h #include sysemu.h diff --git a/hw/pc.c b/hw/pc.c index 6c0722d..805e8ca 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -23,6 +23,7 @@ */ #include hw.h #include pc.h +#include serial.h #include apic.h #include fdc.h #include ide.h diff --git a/hw/pc.h b/hw/pc.h index
[Qemu-devel] [PATCH 7/7] usb-serial: only expose device in guest when the chardev is open
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/usb/dev-serial.c | 19 +-- 1 files changed, 17 insertions(+), 2 deletions(-) diff --git a/hw/usb/dev-serial.c b/hw/usb/dev-serial.c index 43214cd..a466f99 100644 --- a/hw/usb/dev-serial.c +++ b/hw/usb/dev-serial.c @@ -427,6 +427,10 @@ static void usb_serial_handle_destroy(USBDevice *dev) static int usb_serial_can_read(void *opaque) { USBSerialState *s = opaque; + +if (!s-dev.attached) { +return 0; +} return RECV_BUF - s-recv_used; } @@ -469,8 +473,14 @@ static void usb_serial_event(void *opaque, int event) case CHR_EVENT_FOCUS: break; case CHR_EVENT_OPENED: -usb_serial_reset(s); -/* TODO: Reset USB port */ +if (!s-dev.attached) { +usb_device_attach(s-dev); +} +break; +case CHR_EVENT_CLOSED: +if (s-dev.attached) { +usb_device_detach(s-dev); +} break; } } @@ -481,6 +491,7 @@ static int usb_serial_initfn(USBDevice *dev) usb_desc_create_serial(dev); usb_desc_init(dev); +dev-auto_attach = 0; if (!s-cs) { error_report(Property chardev is required); @@ -490,6 +501,10 @@ static int usb_serial_initfn(USBDevice *dev) qemu_chr_add_handlers(s-cs, usb_serial_can_read, usb_serial_read, usb_serial_event, s); usb_serial_handle_reset(dev); + +if (s-cs-opened !dev-attached) { +usb_device_attach(dev); +} return 0; } -- 1.7.1
[Qemu-devel] [PATCH 5/7] serial: add pci-serial documentation
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- docs/specs/pci-serial.txt | 34 ++ hw/serial-pci.c |2 ++ 2 files changed, 36 insertions(+), 0 deletions(-) create mode 100644 docs/specs/pci-serial.txt diff --git a/docs/specs/pci-serial.txt b/docs/specs/pci-serial.txt new file mode 100644 index 000..66c761f --- /dev/null +++ b/docs/specs/pci-serial.txt @@ -0,0 +1,34 @@ + +QEMU pci serial devices +=== + +There is one single-port variant and two muliport-variants. Linux +guests out-of-the box with all cards. There is a Windows inf file +(docs/qemupciserial.inf) to setup the single-port card in Windows +guests. + + +single-port card + + +Name: pci-serial +PCI ID: 1b36:0002 + +PCI Region 0: + IO bar, 8 bytes long, with the 16550 uart mapped to it. + Interrupt is wired to pin A. + + +multiport cards +--- + +Name: pci-serial-2x +PCI ID: 1b36:0003 + +Name: pci-serial-4x +PCI ID: 1b36:0004 + +PCI Region 0: + IO bar, with two/four 16550 uart mapped after each other. + The first is at offset 0, second at offset 8, ... + Interrupt is wired to pin A. diff --git a/hw/serial-pci.c b/hw/serial-pci.c index badd297..95dc5c8 100644 --- a/hw/serial-pci.c +++ b/hw/serial-pci.c @@ -23,6 +23,8 @@ * THE SOFTWARE. */ +/* see docs/specs/pci-serial.txt */ + #include serial.h #include pci.h -- 1.7.1
Re: [Qemu-devel] [Patch]KVM: enabling per domain PLE
The problem with this is that it requires an administrator to understand the workload, not only of the guest, but also of other guests on the machine. With low overcommit, a high PLE window reduces unneeded exits, but with high overcommit we need those exits to reduce spinning. In addition, most kvm hosts don't have an administrator. They are controlled by a management system, which means we'll need some algorithm in userspace to control the PLE window. Taking the two together, we need a dynamic (for changing workloads) algorithm. There are threads discussing this dynamic algorithm, we are making slow progress because it's such a difficult problem, but I think this is much more useful than anything requiring user intervention. Avi, agreed that dynamic adaptive ple should be the best solution. However currently it is a difficult problem like you said. Our solution just gives user a choice who know how to set the two PLE values. So the solution is a compromise solution, which should be better than nothing, for now? :-) Your comments?
Re: [Qemu-devel] [RFC PATCH 0/6] chardev: convert to QOM
Hi, I think the main decision point here is whether we introduce a separate chardev_add/chardev_del command or just use the qom-create command that has been posted previously. Do you have a git tree with this series + qom-create to look at and play with? thanks, Gerd
[Qemu-devel] [PATCH 2/7] serial: add pci variant
So we get a hot-pluggable 16550 uart. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- default-configs/pci.mak |2 + hw/Makefile.objs|1 + hw/pci_ids.h|1 + hw/serial-pci.c | 101 +++ hw/serial.c |6 +++ hw/serial.h |1 + 6 files changed, 112 insertions(+), 0 deletions(-) create mode 100644 hw/serial-pci.c diff --git a/default-configs/pci.mak b/default-configs/pci.mak index 69e18f1..ae9d1eb 100644 --- a/default-configs/pci.mak +++ b/default-configs/pci.mak @@ -19,3 +19,5 @@ CONFIG_IDE_PCI=y CONFIG_AHCI=y CONFIG_ESP=y CONFIG_ESP_PCI=y +CONFIG_SERIAL=y +CONFIG_SERIAL_PCI=y diff --git a/hw/Makefile.objs b/hw/Makefile.objs index 16e7a1e..af4ab0c 100644 --- a/hw/Makefile.objs +++ b/hw/Makefile.objs @@ -21,6 +21,7 @@ common-obj-$(CONFIG_ESCC) += escc.o common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o +common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o common-obj-$(CONFIG_PARALLEL) += parallel.o common-obj-$(CONFIG_I8254) += i8254_common.o i8254.o common-obj-$(CONFIG_PCSPK) += pcspk.o diff --git a/hw/pci_ids.h b/hw/pci_ids.h index 301bf1c..c017a79 100644 --- a/hw/pci_ids.h +++ b/hw/pci_ids.h @@ -37,6 +37,7 @@ #define PCI_CLASS_BRIDGE_PCI 0x0604 #define PCI_CLASS_BRIDGE_OTHER 0x0680 +#define PCI_CLASS_COMMUNICATION_SERIAL 0x0700 #define PCI_CLASS_COMMUNICATION_OTHER0x0780 #define PCI_CLASS_PROCESSOR_CO 0x0b40 diff --git a/hw/serial-pci.c b/hw/serial-pci.c new file mode 100644 index 000..6fcf117 --- /dev/null +++ b/hw/serial-pci.c @@ -0,0 +1,101 @@ +/* + * QEMU 16550A UART emulation + * + * Copyright (c) 2003-2004 Fabrice Bellard + * Copyright (c) 2008 Citrix Systems, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the Software), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#include serial.h +#include pci.h + +typedef struct PCISerialState { +PCIDevice dev; +SerialState state; +} PCISerialState; + +static int serial_pci_init(PCIDevice *dev) +{ +PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev); +SerialState *s = pci-state; + +s-baudbase = 115200; +serial_init_core(s); + +pci-dev.config[PCI_INTERRUPT_PIN] = 0x01; +s-irq = pci-dev.irq[0]; + +memory_region_init_io(s-io, serial_io_ops, s, serial, 8); +pci_register_bar(pci-dev, 0, PCI_BASE_ADDRESS_SPACE_IO, s-io); +return 0; +} + +static void serial_pci_exit(PCIDevice *dev) +{ +PCISerialState *pci = DO_UPCAST(PCISerialState, dev, dev); +SerialState *s = pci-state; + +serial_exit_core(s); +memory_region_destroy(s-io); +} + +static const VMStateDescription vmstate_pci_serial = { +.name = pci-serial, +.version_id = 1, +.minimum_version_id = 1, +.fields = (VMStateField[]) { +VMSTATE_PCI_DEVICE(dev, PCISerialState), +VMSTATE_STRUCT(state, PCISerialState, 0, vmstate_serial, SerialState), +VMSTATE_END_OF_LIST() +} +}; + +static Property serial_pci_properties[] = { +DEFINE_PROP_CHR(chardev, PCISerialState, state.chr), +DEFINE_PROP_END_OF_LIST(), +}; + +static void serial_pci_class_initfn(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass); +pc-init = serial_pci_init; +pc-exit = serial_pci_exit; +pc-vendor_id = 0x1b36; /* Red Hat */ +pc-device_id = 0x0002; +pc-revision = 1; +pc-class_id = PCI_CLASS_COMMUNICATION_SERIAL; +dc-vmsd = vmstate_pci_serial; +dc-props = serial_pci_properties; +} + +static TypeInfo serial_pci_info = { +.name = pci-serial, +.parent= TYPE_PCI_DEVICE, +.instance_size = sizeof(PCISerialState), +.class_init= serial_pci_class_initfn, +}; + +static void serial_pci_register_types(void) +{ +type_register_static(serial_pci_info); +} + +type_init(serial_pci_register_types) diff --git
[Qemu-devel] udev 42-qemu-usb.rules considered harmful?
Hello. I noticed that some versions of linux guests shows quite bad behavour of mouse cursor: it freezes after some idle time, and when you move mouse, it jumps after some delay to rather distant position. So, mouse becomes freezy/jumpy, so to say. Quite some painful debugging pointed to the following udev rule: /lib/udev/rules.d/42-qemu-usb.rules # # Enable autosuspend for qemu emulated usb hid devices. # # Note that there are buggy qemu versions which advertise remote # wakeup support but don't actually implement it correctly. This # is the reason why we need a match for the serial number here. # The serial number 42 is used to tag the implementations where # remote wakeup is working. # ACTION==add, SUBSYSTEM==usb, ATTR{product}==QEMU USB Tablet, ATTR{serial}==42, TEST==power/control, ATTR{power/control}=auto end (And a few other similar rules, for other QEMU USB devices). Removing this rule restores correct mouse behavour. As far as I can see, this rule is to reduce polling interrupt frequency, but apparently it does not help: any usb device connected to guest, and the host CPU usage rises anyway, with or without this rule. Now the question: should we remove this rule? Why it is needed to start with? (qemu version 1.1, but the same bad behavour happens with other versions too). Thanks, /mjt
Re: [Qemu-devel] udev 42-qemu-usb.rules considered harmful?
On 10/17/12 10:41, Michael Tokarev wrote: Hello. I noticed that some versions of linux guests shows quite What is some? As far as I can see, this rule is to reduce polling interrupt frequency, but apparently it does not help: any usb device connected to guest, and the host CPU usage rises anyway, with or without this rule. No, it is for suspending the usb bus, so qemu stops doing 1000 wakeups per second when the usb tablet is idle. Works only in case all devices on the usb bus support it, but for the typical usb use case (-usbdevice tablet for the abs ptr) this is the case. Now the question: should we remove this rule? No. We should fix whatever is broken. Why it is needed to start with? See above. cheers, Gerd
Re: [Qemu-devel] udev 42-qemu-usb.rules considered harmful?
On 17.10.2012 12:56, Gerd Hoffmann wrote: On 10/17/12 10:41, Michael Tokarev wrote: Hello. I noticed that some versions of linux guests shows quite What is some? I guess it is almost all modern distros. Debian wheezy, Ubuntu 12.4, current Fedora (17 I think) to name some. As far as I can see, this rule is to reduce polling interrupt frequency, but apparently it does not help: any usb device connected to guest, and the host CPU usage rises anyway, with or without this rule. No, it is for suspending the usb bus, so qemu stops doing 1000 wakeups per second when the usb tablet is idle. Works only in case all devices This is very close to what I guessed, so I don't see why No ;) on the usb bus support it, but for the typical usb use case (-usbdevice tablet for the abs ptr) this is the case. My command line: qemu-kvm -drive file=linux.img,if=virtio -usbdevice tablet (plus a few other unrelated options). That to say, there's nothing more than just one usb tablet, it is exactly the case mentioned above. And indeed, the CPU utilization drops from - on my host - 12% to about 1.2% after a few secs of mouse being idle in the guest. So I was wrong here, and it actually works - it does what it was supposed to do, it reduces CPU usage. But the mouse is jumpy anyway. I guess one can get used to this behavour, given the tradeoff and that the behavour is not VERY annoying -- it is annoying, but just for a bit, it feels like old mechanical mouse with bad sensor or dirty ball when you can move the mouse but cursor sometimes stays in place and sometimes moves, depending on the surface and direction of the move... ;) No. We should fix whatever is broken. Is it _ever_ possible to fix this? What happens when the (device, bus) is suspended? Does host still sends interrupts periodically (but with much lower frequency)? Or is it the bus/hub/device which should signal attention at some point? Sorry I'm not even a novice in USB internals ;) /mjt
Re: [Qemu-devel] [RFC PATCH v3 06/19] Implement -dimm command line option
On Sat, Oct 13, 2012 at 08:57:19AM +, Blue Swirl wrote: On Tue, Oct 9, 2012 at 5:04 PM, Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com wrote: snip Maybe even the dimmbus device shouldn't exist by itself after all, or it should be pretty much invisible to users. On real HW, the memory controller or south bridge handles the memory. For i440fx, it's part of the same chipset. So I think we should just add qdev properties to i440fx to specify the sizes, nodes etc. Then i440fx should create the dimmbus device unconditionally using the properties. The default properties should create a sane configuration, otherwise -global i440fx.dimm_size=512M etc. could be used. Then the bus would be populated as before or with device_add. hmm the problem with using only i440fx properties, is that size/nodes look dimm specific to me, not chipset-memcontroller specific. Unless we only allow uniform size dimms. Is it possible to have a dynamic list of sizes/nodes pairs as properties of a qdev device? I don't think so, but probably there's a limit of DIMMs that real controllers have, something like 8 max. In the case of i440fx specifically, do you mean that we should model the DRB (Dram row boundary registers in section 3.2.19 of the i440fx spec) ? The i440fx DRB registers only supports up to 8 DRAM rows (let's say 1 row maps 1-1 to a DimmDevice for this discussion) and only supports up to 2GB of memory afaict (bit 31 and above is ignored). I 'd rather not model this part of the i440fx - having only 8 DIMMs seems too restrictive. The rest of the patchset supports up to 255 DIMMs so it would be a waste imho to model an old pc memory controller that only supports 8 DIMMs. There was also an old discussion about i440fx modeling here: https://lists.nongnu.org/archive/html/qemu-devel/2011-07/msg02705.html the general direction was that i440fx is too old and we don't want to precisely emulate the DRB registers, since they lack flexibility. Possible solutions: 1) is there a newer and more flexible chipset that we could model? 2) model and document a generic (non-existent) i440fx that would support more and larger DIMMs. E.g. support 255 DIMMs. If we want to use a description similar to the i440fx DRB registers, the registers would take up a lot of space. In i440fx there is one 8-bit DRB register per DIMM, and DRB[i] describes how many 8MB chunks are contained in DIMMs 0...i. So, the register values are cumulative (and total described memory cannot exceed 256x8MB = 2GB) We could for example model: - an 8-bit non-cumulative register for each DIMM, denoting how many 128MB chunks it contains. This allowes 32GB for each DIMM, and with 255 DIMMs we describe a bit less than 8TB. These registers require 255 bytes. - a 16-bit cumulative register for each DIMM again for 128MB chunks. This allows us to describe 8TB of memory (but the registers take up double the space, because they describe cumulative memory amounts) 3) let everything be handled/abstracted by dimmbus - the chipset DRB modelling is not done (at least for i440fx, other machines could). This is the least precise in terms of emulation. On the other hand, if we are not really trying to emulate the real (too restrictive) hardware, does it matter? thanks, - Vasilis
Re: [Qemu-devel] udev 42-qemu-usb.rules considered harmful?
On 17.10.2012 13:15, Michael Tokarev wrote: But the mouse is jumpy anyway. I guess one can get used to this behavour, given the tradeoff and that the behavour is not VERY annoying -- it is annoying, but just for a bit, it feels like old mechanical mouse with bad sensor or dirty ball when you can move the mouse but cursor sometimes stays in place and sometimes moves, depending on the surface and direction of the move... ;) It is more like the case when your desktop system is swapping heavily, so when you move mouse, it needs to swap-in parts of X or clients which are supposed to react to mouse movement events, so the movement is just delayed, not ignored altogether like with bad/dirty ball. And when you leave it alone, these parts gets swapped out again, to be swapped in when you move it the next time. /mjt
Re: [Qemu-devel] [PATCH v3 9/9] chardev: add hotplug support.
Hi, +- id: the chardev's ID, must be unique (json-string) +- backend: the chardev backend: file, socket, ... (json-string) +- path: file / device / unix socket path (json-string, optional) +- name: spice channel name (json-string, optional) +- host: host name (json-string, optional) +- port: port number (json-string, optional) +- server: create socket in server mode (json-bool, optional) +- wait: wait for connect (json-bool, optional) +- ipv4: force ipv4-only (json-bool, optional) +- ipv6: force ipv6-only (json-bool, optional) +- telnet: telnet negotiation (json-bool, optional) + As mentioned in another thread, should it support the mux argument for multiplexing mode? I don't think so. Basically the only use case for the mux chardev is -nographic with serial line + monitor being multiplexed on stdio. cheers, Gerd
Re: [Qemu-devel] [PATCH v3 0/4] block-commit fixes
Am 16.10.2012 22:33, schrieb Eric Blake: On 10/16/2012 01:49 PM, Jeff Cody wrote: v2 - v3 differences: * Simplified code in patch 2, minor commit rewording * Made conditional check more valgrind-friendly v1 - v2 differences: * Updated changes to be protocol friendly * Updated block-commit tests to account for different error message wording * Updated block-commit tests to add relative backing filename tests * In bdrv_find_backing_image(), use g_malloc() instead of char arrays Jeff Cody (4): block: make bdrv_find_backing_image compare canonical filenames block: in commit, determine base image from the top image qemu-iotests: qemu-io tests update for block-commit (040) qemu-iotests: add relative backing file tests for block-commit (040) Should patch 3/4 be squashed into 2/4 to avoid 'git bisect' failures when running the testsuite? Other than that, series: Yes, I agree. Applied all and squashed 3/4 into 2/4. Kevin
Re: [Qemu-devel] [RFC PATCH v3 06/19] Implement -dimm command line option
On 10/17/2012 11:19 AM, Vasilis Liaskovitis wrote: I don't think so, but probably there's a limit of DIMMs that real controllers have, something like 8 max. In the case of i440fx specifically, do you mean that we should model the DRB (Dram row boundary registers in section 3.2.19 of the i440fx spec) ? The i440fx DRB registers only supports up to 8 DRAM rows (let's say 1 row maps 1-1 to a DimmDevice for this discussion) and only supports up to 2GB of memory afaict (bit 31 and above is ignored). I 'd rather not model this part of the i440fx - having only 8 DIMMs seems too restrictive. The rest of the patchset supports up to 255 DIMMs so it would be a waste imho to model an old pc memory controller that only supports 8 DIMMs. There was also an old discussion about i440fx modeling here: https://lists.nongnu.org/archive/html/qemu-devel/2011-07/msg02705.html the general direction was that i440fx is too old and we don't want to precisely emulate the DRB registers, since they lack flexibility. Possible solutions: 1) is there a newer and more flexible chipset that we could model? Look for q35 on this list. 2) model and document ^--- the critical bit a generic (non-existent) i440fx that would support more and larger DIMMs. E.g. support 255 DIMMs. If we want to use a description similar to the i440fx DRB registers, the registers would take up a lot of space. In i440fx there is one 8-bit DRB register per DIMM, and DRB[i] describes how many 8MB chunks are contained in DIMMs 0...i. So, the register values are cumulative (and total described memory cannot exceed 256x8MB = 2GB) Our i440fx has already been extended by support for pci and cpu hotplug, and I see no reason not to extend it for memory. We can allocate extra mmio space for registers if needed. Usually I'm against this sort of thing, but in this case we don't have much choice. We could for example model: - an 8-bit non-cumulative register for each DIMM, denoting how many 128MB chunks it contains. This allowes 32GB for each DIMM, and with 255 DIMMs we describe a bit less than 8TB. These registers require 255 bytes. - a 16-bit cumulative register for each DIMM again for 128MB chunks. This allows us to describe 8TB of memory (but the registers take up double the space, because they describe cumulative memory amounts) There is no reason to save space. Why not have two 64-bit registers per DIMM, one describing the size and the other the base address, both in bytes? Use a few low order bits for control. 3) let everything be handled/abstracted by dimmbus - the chipset DRB modelling is not done (at least for i440fx, other machines could). This is the least precise in terms of emulation. On the other hand, if we are not really trying to emulate the real (too restrictive) hardware, does it matter? We could emulate base memory using the chipset, and extra memory using the scheme above. This allows guests that are tied to the chipset to work, and guests that have more awareness (seabios) to use the extra features. -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH 0/3] chardev hotplug patch series
Hi, Here comes the updated chardev hotplug patch series. Addressed most review comments. It's chardev-add and chardev-remove now. Parameters have been added to the schema. Little nits and spell fixes here and there. Making 'backend' an enum is tricky given that the actual implementation just turns the qdict into a QemuOpts, then goes piggyback on qemu_chr_new_from_opts(). Not rebased (yet) on top of the qom queue as I expect more discussions. please review, Gerd Gerd Hoffmann (3): chardev: add error reporting for qemu_chr_new_from_opts chardev: fix QemuOpts lifecycle chardev: add hotplug support. hmp-commands.hx | 32 + hmp.c| 23 +++ hmp.h|2 + qapi-schema.json | 47 ++ qemu-char.c | 83 - qemu-char.h |5 ++- qmp-commands.hx | 61 +++ vl.c |8 - 8 files changed, 244 insertions(+), 17 deletions(-)
[Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- qemu-char.c | 24 +++- qemu-char.h |3 ++- vl.c|8 ++-- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index b082bae..e2e1da8 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2755,19 +2755,20 @@ static const struct { }; CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, -void (*init)(struct CharDriverState *s)) +void (*init)(struct CharDriverState *s), +Error **errp) { CharDriverState *chr; int i; if (qemu_opts_id(opts) == NULL) { -fprintf(stderr, chardev: no id specified\n); +error_setg(errp, chardev: no id specified\n); return NULL; } if (qemu_opt_get(opts, backend) == NULL) { -fprintf(stderr, chardev: \%s\ missing backend\n, -qemu_opts_id(opts)); +error_setg(errp, chardev: \%s\ missing backend\n, + qemu_opts_id(opts)); return NULL; } for (i = 0; i ARRAY_SIZE(backend_table); i++) { @@ -2775,15 +2776,15 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, break; } if (i == ARRAY_SIZE(backend_table)) { -fprintf(stderr, chardev: backend \%s\ not found\n, -qemu_opt_get(opts, backend)); +error_setg(errp, chardev: backend \%s\ not found\n, + qemu_opt_get(opts, backend)); return NULL; } chr = backend_table[i].open(opts); if (!chr) { -fprintf(stderr, chardev: opening backend \%s\ failed\n, -qemu_opt_get(opts, backend)); +error_setg(errp, chardev: opening backend \%s\ failed\n, + qemu_opt_get(opts, backend)); return NULL; } @@ -2813,6 +2814,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in const char *p; CharDriverState *chr; QemuOpts *opts; +Error *err = NULL; if (strstart(filename, chardev:, p)) { return qemu_chr_find(p); @@ -2822,7 +2824,11 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in if (!opts) return NULL; -chr = qemu_chr_new_from_opts(opts, init); +chr = qemu_chr_new_from_opts(opts, init, err); +if (error_is_set(err)) { +fprintf(stderr, %s\n, error_get_pretty(err)); +error_free(err); +} if (chr qemu_opt_get_bool(opts, mux, 0)) { monitor_init(chr, MONITOR_USE_READLINE); } diff --git a/qemu-char.h b/qemu-char.h index 486644b..adae13e 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -88,7 +88,8 @@ struct CharDriverState { * Returns: a new character backend */ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, -void (*init)(struct CharDriverState *s)); +void (*init)(struct CharDriverState *s), +Error **errp); /** * @qemu_chr_new: diff --git a/vl.c b/vl.c index 5b357a3..d2f1c10 100644 --- a/vl.c +++ b/vl.c @@ -1940,10 +1940,14 @@ static int device_init_func(QemuOpts *opts, void *opaque) static int chardev_init_func(QemuOpts *opts, void *opaque) { CharDriverState *chr; +Error *local_err = NULL; -chr = qemu_chr_new_from_opts(opts, NULL); -if (!chr) +chr = qemu_chr_new_from_opts(opts, NULL, local_err); +if (error_is_set(local_err)) { +fprintf(stderr, %s\n, error_get_pretty(local_err)); +error_free(local_err); return -1; +} return 0; } -- 1.7.1
[Qemu-devel] [PATCH 2/3] chardev: fix QemuOpts lifecycle
qemu_chr_new_from_opts handles QemuOpts release now, so callers don't have to worry. It will either be saved in CharDriverState, then released in qemu_chr_delete, or in the error case released instantly. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- qemu-char.c | 15 ++- qemu-char.h |1 + 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index e2e1da8..be4ec61 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2763,13 +2763,13 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, if (qemu_opts_id(opts) == NULL) { error_setg(errp, chardev: no id specified\n); -return NULL; +goto err; } if (qemu_opt_get(opts, backend) == NULL) { error_setg(errp, chardev: \%s\ missing backend\n, qemu_opts_id(opts)); -return NULL; +goto err; } for (i = 0; i ARRAY_SIZE(backend_table); i++) { if (strcmp(backend_table[i].name, qemu_opt_get(opts, backend)) == 0) @@ -2778,14 +2778,14 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, if (i == ARRAY_SIZE(backend_table)) { error_setg(errp, chardev: backend \%s\ not found\n, qemu_opt_get(opts, backend)); -return NULL; +goto err; } chr = backend_table[i].open(opts); if (!chr) { error_setg(errp, chardev: opening backend \%s\ failed\n, qemu_opt_get(opts, backend)); -return NULL; +goto err; } if (!chr-filename) @@ -2806,7 +2806,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, chr-avail_connections = 1; } chr-label = g_strdup(qemu_opts_id(opts)); +chr-opts = opts; return chr; + +err: +qemu_opts_del(opts); +return NULL; } CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*init)(struct CharDriverState *s)) @@ -2832,7 +2837,6 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in if (chr qemu_opt_get_bool(opts, mux, 0)) { monitor_init(chr, MONITOR_USE_READLINE); } -qemu_opts_del(opts); return chr; } @@ -2864,6 +2868,7 @@ void qemu_chr_delete(CharDriverState *chr) chr-chr_close(chr); g_free(chr-filename); g_free(chr-label); +qemu_opts_del(chr-opts); g_free(chr); } diff --git a/qemu-char.h b/qemu-char.h index adae13e..99bc132 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -74,6 +74,7 @@ struct CharDriverState { char *filename; int opened; int avail_connections; +QemuOpts *opts; QTAILQ_ENTRY(CharDriverState) next; }; -- 1.7.1
Re: [Qemu-devel] [Patch]KVM: enabling per domain PLE
On 10/17/2012 10:02 AM, Hu, Xuekun wrote: The problem with this is that it requires an administrator to understand the workload, not only of the guest, but also of other guests on the machine. With low overcommit, a high PLE window reduces unneeded exits, but with high overcommit we need those exits to reduce spinning. In addition, most kvm hosts don't have an administrator. They are controlled by a management system, which means we'll need some algorithm in userspace to control the PLE window. Taking the two together, we need a dynamic (for changing workloads) algorithm. There are threads discussing this dynamic algorithm, we are making slow progress because it's such a difficult problem, but I think this is much more useful than anything requiring user intervention. Avi, agreed that dynamic adaptive ple should be the best solution. However currently it is a difficult problem like you said. Our solution just gives user a choice who know how to set the two PLE values. So the solution is a compromise solution, which should be better than nothing, for now? :-) Let's see how the PLE thread works out. Yes the patches give the user control, but we need to make sure the user knows how to control it (in fact your patch doesn't even update the documentation). Just throwing out a new ioctl, even if it is documented, doesn't mean that userspace will begin to use it, or that users will exploit it. Do you have a specific use case in mind? -- error compiling committee.c: too many arguments to function
[Qemu-devel] [PATCH 3/3] chardev: add hotplug support.
This patch adds chardev_add and chardev_remove monitor commands. They work similar to the netdev_{add,del} commands. The hmp version of chardev_add accepts like the -chardev command line option does. The qmp version expects the arguments being passed as named parameters. chardev_del just takes an id argument and zaps the chardev specified. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hmp-commands.hx | 32 hmp.c| 23 hmp.h|2 + qapi-schema.json | 47 + qemu-char.c | 44 ++ qemu-char.h |1 + qmp-commands.hx | 61 ++ 7 files changed, 210 insertions(+), 0 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index e0b537d..5207383 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1404,6 +1404,38 @@ passed since 1970, i.e. unix epoch. ETEXI { +.name = chardev_add, +.args_type = args:s, +.params = args, +.help = add chardev, +.mhandler.cmd = hmp_chardev_add, +}, + +STEXI +@item chardev_add args +@findex chardev_add + +chardev_add accepts the same parameters as the -chardev command line switch. + +ETEXI + +{ +.name = chardev_remove, +.args_type = id:s, +.params = id, +.help = remove chardev, +.mhandler.cmd = hmp_chardev_remove, +}, + +STEXI +@item chardev_remove id +@findex chardev_remove + +Removes the chardev @var{id}. + +ETEXI + +{ .name = info, .args_type = item:s?, .params = [subcommand], diff --git a/hmp.c b/hmp.c index 70bdec2..fc9d7bc 100644 --- a/hmp.c +++ b/hmp.c @@ -1209,3 +1209,26 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict) qmp_screendump(filename, err); hmp_handle_error(mon, err); } + +void hmp_chardev_add(Monitor *mon, const QDict *qdict) +{ +const char *args = qdict_get_str(qdict, args); +Error *err = NULL; +QemuOpts *opts; + +opts = qemu_opts_parse(qemu_find_opts(chardev), args, 1); +if (opts == NULL) { +error_setg(err, Parsing chardev args failed\n); +} else { +qemu_chr_new_from_opts(opts, NULL, err); +} +hmp_handle_error(mon, err); +} + +void hmp_chardev_remove(Monitor *mon, const QDict *qdict) +{ +Error *err = NULL; +qmp_chardev_remove(qdict_get_str(qdict, id), + err); +hmp_handle_error(mon, err); +} diff --git a/hmp.h b/hmp.h index 71ea384..cabe944 100644 --- a/hmp.h +++ b/hmp.h @@ -75,5 +75,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict); void hmp_closefd(Monitor *mon, const QDict *qdict); void hmp_send_key(Monitor *mon, const QDict *qdict); void hmp_screen_dump(Monitor *mon, const QDict *qdict); +void hmp_chardev_add(Monitor *mon, const QDict *qdict); +void hmp_chardev_remove(Monitor *mon, const QDict *qdict); #endif diff --git a/qapi-schema.json b/qapi-schema.json index f9dbdae..fab6bbc 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2796,3 +2796,50 @@ # Since: 0.14.0 ## { 'command': 'screendump', 'data': {'filename': 'str'} } + +## +# @chardev-add: +# +# Add a chardev +# +# @id: the chardev's ID, must be unique +# @backend: the chardev backend: file, socket, ... +# @path: file / device / unix socket path +# @name: spice channel name +# @host: host name +# @port: port number +# @server: create socket in server mode +# @wait: wait for connect +# @ipv4: force ipv4-only +# @ipv6: force ipv6-only +# @telnet: telnet negotiation +# +# Returns: Nothing on success +# +# Since: 1.3.0 +## +{ 'command': 'chardev-add', 'data': {'id' : 'str', + 'backend' : 'str', + '*path' : 'str', + '*name' : 'str', + '*host' : 'str', + '*port' : 'str', + '*server' : 'bool', + '*wait' : 'bool', + '*ipv4' : 'bool', + '*ipv6' : 'bool', + '*telnet' : 'bool' }, + 'gen': 'no' } + +## +# @chardev-remove: +# +# Remove a chardev +# +# @id: the chardev's ID, must exist and not be in use +# +# Returns: Nothing on success +# +# Since: 1.3.0 +## +{ 'command': 'chardev-remove', 'data': {'id': 'str'} } diff --git a/qemu-char.c b/qemu-char.c index be4ec61..cbfa24e 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2911,3 +2911,47 @@ CharDriverState *qemu_char_get_next_serial(void) return serial_hds[next_serial++]; } +int qmp_chardev_add(Monitor *mon, const QDict *qdict, QObject **ret) +{ +Error *err = NULL; +QemuOptsList *opts_list; +QemuOpts *opts; + +opts_list =
Re: [Qemu-devel] udev 42-qemu-usb.rules considered harmful?
Hi, But the mouse is jumpy anyway. I guess one can get used to this behavour, given the tradeoff and that the behavour is not VERY annoying -- it is annoying, but just for a bit, it feels like old mechanical mouse with bad sensor or dirty ball when you can move the mouse but cursor sometimes stays in place and sometimes moves, depending on the surface and direction of the move... ;) How long are the delays for you? What happens when the (device, bus) is suspended? Does host still sends interrupts periodically (but with much lower frequency)? Or is it the bus/hub/device which should signal attention at some point? UHCI raises an IRQ, guest restarts USB bus in response. Which takes a bit of time, so the first mouse event of an idle tablet takes a little bit longer. As far I know linux schedules the first poll a bit in the future, so this could be where the delay comes from. Do you also see delays with xhci? (-device nec-pci-xhci -device usb-tablet) ? cheers, Gerd
Re: [Qemu-devel] [PATCH 10/10] tcg: Optimize mulu2
On 10/17/2012 03:09 AM, Richard Henderson wrote: On 2012-10-17 09:25, Aurelien Jarno wrote: +gen_opc_buf[op_index] = op = INDEX_op_mul_i32; Very minor nitpick: you probably don't need to set op there. Perhaps not, but I prefer to keep the variables in sync as we drop into common code... The compiler should recognize the dead variable anyway. How very meta. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 06/22] ehci: Speed up the timer of raising int from the async schedule
On 10/15/12 15:00, Hans de Goede wrote: Hi, On 10/15/2012 01:17 PM, Gerd Hoffmann wrote: On 10/15/12 12:38, Hans de Goede wrote: Often the guest will queue up new packets in response to a packet, in the async schedule with its IOC flag set, completing. By speeding up the frame-timer, we notice these new packets earlier. This increases the speed (MB/s) of a Linux guest reading from a USB mass storage device by a factor of 1.15 on top of the Improve latency of interrupt delivery speed-ups, both with and without input pipelining enabled. Why not just set async_stepdown to 0? We already do that whenever we run a package completion (it get sets when we move to the executing stage). What this patch does is request the frame timer to run again in 500 usecs instead of after 1 ms, thus making us see and process async transfers faster when they are queued up in response to just completed packages (which we've told the guest about with the int interrupt). This makes the USB-bus / device idle time between any 2 transfers of the 3 transfer involving USB storage BOT time shorter, thereby speeding things up. Don't feel like having two mechanisms for wakeup rate control. Can't we integrate this with async_stepdown? Changing the baseline maybe, so stepdown=0 doesn't mean 1000 Hz but 2000 Hz? cheers, Gerd
Re: [Qemu-devel] [PATCH 13/22] usb: Add an int_req flag to USBPacket
On 10/15/12 12:38, Hans de Goede wrote: -usb_packet_setup(xfer-packet, dir, ep, xfer-trbs[0].addr, false); +int_req = false; +for (i = 0; i xfer-trb_count; i++) { +if (xfer-trbs[i].control TRB_TR_IOC) { +int_req = true; +break; +} +} Guess we better add a new int_req field to XHCITransfer and fill it when walking the trb list _anyway_, so we don't have to loop again. cheers, Gerd
Re: [Qemu-devel] [PATCH 06/22] ehci: Speed up the timer of raising int from the async schedule
Hi, On 10/17/2012 01:01 PM, Gerd Hoffmann wrote: On 10/15/12 15:00, Hans de Goede wrote: Hi, On 10/15/2012 01:17 PM, Gerd Hoffmann wrote: On 10/15/12 12:38, Hans de Goede wrote: Often the guest will queue up new packets in response to a packet, in the async schedule with its IOC flag set, completing. By speeding up the frame-timer, we notice these new packets earlier. This increases the speed (MB/s) of a Linux guest reading from a USB mass storage device by a factor of 1.15 on top of the Improve latency of interrupt delivery speed-ups, both with and without input pipelining enabled. Why not just set async_stepdown to 0? We already do that whenever we run a package completion (it get sets when we move to the executing stage). What this patch does is request the frame timer to run again in 500 usecs instead of after 1 ms, thus making us see and process async transfers faster when they are queued up in response to just completed packages (which we've told the guest about with the int interrupt). This makes the USB-bus / device idle time between any 2 transfers of the 3 transfer involving USB storage BOT time shorter, thereby speeding things up. Don't feel like having two mechanisms for wakeup rate control. Can't we integrate this with async_stepdown? Changing the baseline maybe, so stepdown=0 doesn't mean 1000 Hz but 2000 Hz? That is actually close to what I wanted to do at first (I wanted to use stepdown=-1 for the faster wakeup case). But there are 2 problems with this: 1) It causes migration issues when migrating to / from an old version 2) We don't want to change the wakeup rate when the interrupt flag gets set as pending, but when it actually gets committed, and we only want to change the wakeup rate when the int was requested by an async packet, not when it was requested by a periodic packet, so we will need the int_req_by_async flag anyways, as which point this seemed the cleanest way. Regards, Hans
Re: [Qemu-devel] [PATCH 13/22] usb: Add an int_req flag to USBPacket
Hi, On 10/17/2012 01:04 PM, Gerd Hoffmann wrote: On 10/15/12 12:38, Hans de Goede wrote: -usb_packet_setup(xfer-packet, dir, ep, xfer-trbs[0].addr, false); +int_req = false; +for (i = 0; i xfer-trb_count; i++) { +if (xfer-trbs[i].control TRB_TR_IOC) { +int_req = true; +break; +} +} Guess we better add a new int_req field to XHCITransfer and fill it when walking the trb list _anyway_, so we don't have to loop again. Ok, I will fix this for the next respin of the patchset. Regards, Hans
Re: [Qemu-devel] [PATCH 14/22] usb: Add packet combining functions
Hi, +/* + * Process / cancel combined packets, called from + * usb_ep_combine_input_packets() / usb_combined_packet_cancel(). + * Only called for devices which call these functions themselves. + */ +int (*handle_combined_data)(USBDevice *dev, USBPacket *p); +void (*cancel_combined_packet)(USBDevice *dev, USBPacket *p); Do we really need these? I think it isn't much work for the callers to do that themself. Saves them providing a callback. And makes the code flow easier to follow by removing a pointless indirection. For handle_combined_data we probably must make usb_ep_combine_input_packets return a status code. cheers, Gerd
Re: [Qemu-devel] [PATCH 06/22] ehci: Speed up the timer of raising int from the async schedule
Hi, 1) It causes migration issues when migrating to / from an old version With -1 yes, shifting the scale shoudn't be a that big issue though as it is just a optimization. 2) We don't want to change the wakeup rate when the interrupt flag gets set as pending, but when it actually gets committed, and we only want to change the wakeup rate when the int was requested by an async packet, not when it was requested by a periodic packet, so we will need the int_req_by_async flag anyways, as which point this seemed the cleanest way. Missed that little details, ok then. cheers, Gerd
[Qemu-devel] [PATCH 1/2] microblaze: Support setting of TLS ptr
From: Edgar E. Iglesias edgar.igles...@gmail.com Signed-off-by: Edgar E. Iglesias edgar.igles...@gmail.com --- linux-user/syscall.c|2 ++ target-microblaze/cpu.h |1 + 2 files changed, 3 insertions(+), 0 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 471d060..c6a6337 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -6888,6 +6888,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, ret = get_errno(do_fork(cpu_env, arg1, arg2, arg3, arg5, arg4)); #elif defined(TARGET_CRIS) ret = get_errno(do_fork(cpu_env, arg2, arg1, arg3, arg4, arg5)); +#elif defined(TARGET_MICROBLAZE) +ret = get_errno(do_fork(cpu_env, arg1, arg2, arg4, arg6, arg5)); #elif defined(TARGET_S390X) ret = get_errno(do_fork(cpu_env, arg2, arg1, arg3, arg5, arg4)); #else diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h index 4968c24..88430b5 100644 --- a/target-microblaze/cpu.h +++ b/target-microblaze/cpu.h @@ -345,6 +345,7 @@ static inline void cpu_clone_regs(CPUMBState *env, target_ulong newsp) static inline void cpu_set_tls(CPUMBState *env, target_ulong newtls) { +env-regs[21] = newtls; } static inline int cpu_interrupts_enabled(CPUMBState *env) -- 1.7.8.6
[Qemu-devel] [PATCH 2/2] microblaze: Update PC before simulating syscall
From: Edgar E. Iglesias edgar.igles...@gmail.com Fixes a clone() emulation bug were the new thread starts at the point of the syscall and thus clones in a loop. Signed-off-by: Edgar E. Iglesias edgar.igles...@gmail.com --- linux-user/main.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/linux-user/main.c b/linux-user/main.c index f4bbe69..5827ee6 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -2527,6 +2527,7 @@ void cpu_loop(CPUMBState *env) case EXCP_BREAK: /* Return address is 4 bytes after the call. */ env-regs[14] += 4; +env-sregs[SR_PC] = env-regs[14]; ret = do_syscall(env, env-regs[12], env-regs[5], @@ -2537,7 +2538,6 @@ void cpu_loop(CPUMBState *env) env-regs[10], 0, 0); env-regs[3] = ret; -env-sregs[SR_PC] = env-regs[14]; break; case EXCP_HW_EXCP: env-regs[17] = env-sregs[SR_PC] + 4; -- 1.7.8.6
[Qemu-devel] [PATCH v3 1/2] qemu-img: Add --backing-chain option to info command
The qemu-img info --backing-chain option enumerates the backing file chain. For example, for base.qcow2 - snap1.qcow2 - snap2.qcow2 the output becomes: $ qemu-img info --backing-chain snap2.qcow2 image: snap2.qcow2 file format: qcow2 virtual size: 100M (104857600 bytes) disk size: 196K cluster_size: 65536 backing file: snap1.qcow2 backing file format: qcow2 image: snap1.qcow2 file format: qcow2 virtual size: 100M (104857600 bytes) disk size: 196K cluster_size: 65536 backing file: base.qcow2 backing file format: qcow2 image: base.qcow2 file format: qcow2 virtual size: 100M (104857600 bytes) disk size: 136K cluster_size: 65536 Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- qemu-img.c | 167 +++-- 1 file changed, 153 insertions(+), 14 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index f17f187..7261be9 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1108,6 +1108,23 @@ static void dump_snapshots(BlockDriverState *bs) g_free(sn_tab); } +static void dump_json_image_info_list(ImageInfoList *list) +{ +Error *errp = NULL; +QString *str; +QmpOutputVisitor *ov = qmp_output_visitor_new(); +QObject *obj; +visit_type_ImageInfoList(qmp_output_get_visitor(ov), + list, NULL, errp); +obj = qmp_output_get_qobject(ov); +str = qobject_to_json_pretty(obj); +assert(str != NULL); +printf(%s\n, qstring_get_str(str)); +qobject_decref(obj); +qmp_output_visitor_cleanup(ov); +QDECREF(str); +} + static void collect_snapshots(BlockDriverState *bs , ImageInfo *info) { int i, sn_count; @@ -1247,9 +1264,129 @@ static void dump_human_image_info(ImageInfo *info) printf(backing file format: %s\n, info-backing_filename_format); } } + +if (info-has_snapshots) { +SnapshotInfoList *elem; +char buf[256]; + +printf(Snapshot list:\n); +printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), NULL)); + +/* Ideally bdrv_snapshot_dump() would operate on SnapshotInfoList but + * we convert to the block layer's native QEMUSnapshotInfo for now. + */ +for (elem = info-snapshots; elem; elem = elem-next) { +QEMUSnapshotInfo sn = { +.vm_state_size = elem-value-vm_state_size, +.date_sec = elem-value-date_sec, +.date_nsec = elem-value-date_nsec, +.vm_clock_nsec = elem-value-vm_clock_sec * 10ULL + + elem-value-vm_clock_nsec, +}; + +pstrcpy(sn.id_str, sizeof(sn.id_str), elem-value-id); +pstrcpy(sn.name, sizeof(sn.name), elem-value-name); +printf(%s\n, bdrv_snapshot_dump(buf, sizeof(buf), sn)); +} +} +} + +static void dump_human_image_info_list(ImageInfoList *list) +{ +ImageInfoList *elem; +bool delim = false; + +for (elem = list; elem; elem = elem-next) { +if (delim) { +printf(\n); +} +delim = true; + +dump_human_image_info(elem-value); +} } -enum {OPTION_OUTPUT = 256}; +static gboolean str_equal_func(gconstpointer a, gconstpointer b) +{ +return strcmp(a, b) == 0; +} + +/** + * Open an image file chain and return an ImageInfoList + * + * @filename: topmost image filename + * @fmt: topmost image format (may be NULL to autodetect) + * @chain: true - enumerate entire backing file chain + * false - only topmost image file + * + * Returns a list of ImageInfo objects or NULL if there was an error opening an + * image file. If there was an error a message will have been printed to + * stderr. + */ +static ImageInfoList *collect_image_info_list(const char *filename, + const char *fmt, + bool chain) +{ +ImageInfoList *head = NULL; +ImageInfoList **last = head; +GHashTable *filenames; + +filenames = g_hash_table_new_full(g_str_hash, str_equal_func, NULL, NULL); + +while (filename) { +BlockDriverState *bs; +ImageInfo *info; +ImageInfoList *elem; + +if (g_hash_table_lookup_extended(filenames, filename, NULL, NULL)) { +error_report(Backing file '%s' creates an infinite loop., + filename); +goto err; +} +g_hash_table_insert(filenames, (gpointer)filename, NULL); + +bs = bdrv_new_open(filename, fmt, BDRV_O_FLAGS | BDRV_O_NO_BACKING, + false); +if (!bs) { +goto err; +} + +info = g_new0(ImageInfo, 1); +collect_image_info(bs, info, filename, fmt); +collect_snapshots(bs, info); + +elem = g_new0(ImageInfoList, 1); +elem-value = info; +*last = elem; +last = elem-next; + +bdrv_delete(bs); + +filename = fmt = NULL; +
[Qemu-devel] [PATCH v3 0/2] qemu-img: Add --backing-chain option to info command
This series adds the --backing-chain option for enumerating the backing file chain. Given the topmost image it will print qemu-img info information for each image file in the chain. Special care needs to be taken when image files form an infinite loop. This is very unusual, most like due to malicious image files. Nevertheless, qemu-img must be robust against invalid inputs so we explicit check for this. v3: * Use ImageInfoList to avoid manually outputting JSON and to make the code more QAPI-friendly. * Solve the output vs error messages issues by printing a detailed error message but no image info. To dig into a broken image chain, rerun without --backing-chain. This is different from what I initially tried but is much cleaner because it doesn't produce confusing output. * rm -f $TEST_IMG.[123].base [Kevin] * Include --output=json in test case [Kevin] * Rename test case to free 043 number [Eric] Stefan Hajnoczi (2): qemu-img: Add --backing-chain option to info command qemu-iotests: Add 043 backing file chain infinite loop test qemu-img.c | 167 +++ tests/qemu-iotests/043 | 94 tests/qemu-iotests/043.out | 66 + tests/qemu-iotests/common.rc | 10 +++ tests/qemu-iotests/group | 1 + 5 files changed, 324 insertions(+), 14 deletions(-) create mode 100755 tests/qemu-iotests/043 create mode 100644 tests/qemu-iotests/043.out -- 1.7.11.7
[Qemu-devel] [PATCH v3 2/2] qemu-iotests: Add 043 backing file chain infinite loop test
This new test verifies that qemu-img info --backing-chain safely aborts when an image file has a backing file infinite loop. Signed-off-by: Stefan Hajnoczi stefa...@redhat.com --- tests/qemu-iotests/043 | 94 tests/qemu-iotests/043.out | 66 +++ tests/qemu-iotests/common.rc | 10 + tests/qemu-iotests/group | 1 + 4 files changed, 171 insertions(+) create mode 100755 tests/qemu-iotests/043 create mode 100644 tests/qemu-iotests/043.out diff --git a/tests/qemu-iotests/043 b/tests/qemu-iotests/043 new file mode 100755 index 000..bf390ac --- /dev/null +++ b/tests/qemu-iotests/043 @@ -0,0 +1,94 @@ +#!/bin/bash +# +# Test that qemu-img info --backing-chain detects infinite loops +# +# Copyright (C) 2012 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see http://www.gnu.org/licenses/. +# + +# creator +owner=stefa...@redhat.com + +seq=`basename $0` +echo QA output created by $seq + +here=`pwd` +tmp=/tmp/$$ +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap _cleanup; exit \$status 0 1 2 3 15 + +# get standard environment, filters and checks +. ./common.rc +. ./common.filter + +# Any format supporting backing files +_supported_fmt qcow qcow2 vmdk qed +_supported_proto generic +_supported_os Linux + + +size=128M +_make_test_img $size +$QEMU_IMG rebase -u -b $TEST_IMG $TEST_IMG + +echo +echo == backing file references self == +_img_info --backing-chain + +_make_test_img $size +mv $TEST_IMG $TEST_IMG.base +_make_test_img -b $TEST_IMG.base $size +$QEMU_IMG rebase -u -b $TEST_IMG $TEST_IMG.base + +echo +echo == parent references self == +_img_info --backing-chain + +_make_test_img $size +mv $TEST_IMG $TEST_IMG.1.base +_make_test_img -b $TEST_IMG.1.base $size +mv $TEST_IMG $TEST_IMG.2.base +_make_test_img -b $TEST_IMG.2.base $size +mv $TEST_IMG $TEST_IMG.3.base +_make_test_img -b $TEST_IMG.3.base $size +$QEMU_IMG rebase -u -b $TEST_IMG.2.base $TEST_IMG.1.base + +echo +echo == ancestor references another ancestor == +_img_info --backing-chain + +_make_test_img $size +mv $TEST_IMG $TEST_IMG.1.base +_make_test_img -b $TEST_IMG.1.base $size +mv $TEST_IMG $TEST_IMG.2.base +_make_test_img -b $TEST_IMG.2.base $size + +echo +echo == finite chain of length 3 (human) == +_img_info --backing-chain + +echo +echo == finite chain of length 3 (json) == +_img_info --backing-chain --output=json + +# success, all done +echo *** done +rm -f $seq.full $TEST_IMG.[123].base +status=0 diff --git a/tests/qemu-iotests/043.out b/tests/qemu-iotests/043.out new file mode 100644 index 000..ad23337 --- /dev/null +++ b/tests/qemu-iotests/043.out @@ -0,0 +1,66 @@ +QA output created by 043 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 + +== backing file references self == +qemu-img: Backing file 'TEST_DIR/t.IMGFMT' creates an infinite loop. +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file='TEST_DIR/t.IMGFMT.base' + +== parent references self == +qemu-img: Backing file 'TEST_DIR/t.IMGFMT' creates an infinite loop. +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file='TEST_DIR/t.IMGFMT.1.base' +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file='TEST_DIR/t.IMGFMT.2.base' +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file='TEST_DIR/t.IMGFMT.3.base' + +== ancestor references another ancestor == +qemu-img: Backing file 'TEST_DIR/t.IMGFMT.2.base' creates an infinite loop. +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file='TEST_DIR/t.IMGFMT.1.base' +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 backing_file='TEST_DIR/t.IMGFMT.2.base' + +== finite chain of length 3 (human) == +image: TEST_DIR/t.IMGFMT +file format: IMGFMT +virtual size: 128M (134217728 bytes) +cluster_size: 65536 +backing file: TEST_DIR/t.IMGFMT.2.base + +image: TEST_DIR/t.IMGFMT.2.base +file format: IMGFMT +virtual size: 128M (134217728 bytes) +cluster_size: 65536 +backing file: TEST_DIR/t.IMGFMT.1.base + +image: TEST_DIR/t.IMGFMT.1.base +file format: IMGFMT +virtual size: 128M (134217728 bytes) +cluster_size: 65536 + +== finite chain of length 3 (json) == +[ +{
Re: [Qemu-devel] [PATCH 01/18] error: add error_set_errno and error_setg_errno
Luiz Capitulino lcapitul...@redhat.com writes: On Thu, 04 Oct 2012 18:16:13 +0200 Paolo Bonzini pbonz...@redhat.com wrote: Il 04/10/2012 18:14, Luiz Capitulino ha scritto: +void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, + const char *fmt, ...) The function's name makes me expect that something else is done with os_errno Why something else? :) What I meant is that, it's not clear from the function's name how os_errno is used. Actually, it gives the impression you're storing errno, but I might be biased :) But again, I don't have any better suggestions. A bit long for my taste, but here goes anyway: error_set_with_errno()
Re: [Qemu-devel] [PATCH qom-cpu v2 1/7] target-i386: Inline APIC cpu_env property setting
On 2012-10-16 18:02, Andreas Färber wrote: Am 12.10.2012 03:26, schrieb Andreas Färber: This prepares for changing the variable type from void*. Signed-off-by: Andreas Färber afaer...@suse.de Cc: Igor Mammedov imamm...@redhat.com Paolo, are you happy with getting rid of the pointer property this way? Jan, are you okay with accessing APICCommonState in target-i386/cpu.c as an interim solution? Yes, I don't see it as a problem. Jan Andreas --- hw/apic_common.c |1 - target-i386/cpu.c |5 - 2 Dateien geändert, 4 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-) diff --git a/hw/apic_common.c b/hw/apic_common.c index 371f95d..a26a631 100644 --- a/hw/apic_common.c +++ b/hw/apic_common.c @@ -368,7 +368,6 @@ static const VMStateDescription vmstate_apic_common = { static Property apic_properties_common[] = { DEFINE_PROP_UINT8(id, APICCommonState, id, -1), -DEFINE_PROP_PTR(cpu_env, APICCommonState, cpu_env), DEFINE_PROP_BIT(vapic, APICCommonState, vapic_control, VAPIC_ENABLE_BIT, true), DEFINE_PROP_END_OF_LIST(), diff --git a/target-i386/cpu.c b/target-i386/cpu.c index e307b4e..0cce910 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -41,6 +41,7 @@ #ifndef CONFIG_USER_ONLY #include hw/xen.h #include hw/sysbus.h +#include hw/apic_internal.h #endif /* feature flags taken from Intel Processor Identification and the CPUID @@ -1884,6 +1885,7 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) #ifndef CONFIG_USER_ONLY static int apic_mapped; CPUX86State *env = cpu-env; +APICCommonState *apic; const char *apic_type = apic; if (kvm_irqchip_in_kernel()) { @@ -1902,7 +1904,8 @@ static void x86_cpu_apic_init(X86CPU *cpu, Error **errp) OBJECT(env-apic_state), NULL); qdev_prop_set_uint8(env-apic_state, id, env-cpuid_apic_id); /* TODO: convert to link */ -qdev_prop_set_ptr(env-apic_state, cpu_env, env); +apic = APIC_COMMON(env-apic_state); +apic-cpu_env = env; if (qdev_init(env-apic_state)) { error_setg(errp, APIC device '%s' could not be initialized, signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/2] hmp: fix info cpus for sparc targets
On Wed, 17 Oct 2012 08:06:13 +0200 Aurelien Jarno aurel...@aurel32.net wrote: On Tue, Oct 16, 2012 at 11:01:45PM -0300, Luiz Capitulino wrote: On Wed, 17 Oct 2012 01:28:34 +0200 Aurelien Jarno aurel...@aurel32.net wrote: On sparc targets, info cpus returns this kind of output: | info cpus | * CPU #0: pc=0x00424d18pc=0x00424d18npc=0x00424d1c thread_id=19460 pc is printed twice, there is no space between pc, pc and npc. As npc implies pc, don't set has_pc on sparc, and add a space between pc and npc. Cc: Luiz Capitulino lcapitul...@redhat.com Cc: Markus Armbruster arm...@redhat.com Cc: Blue Swirl blauwir...@gmail.com Signed-off-by: Aurelien Jarno aurel...@aurel32.net --- cpus.c |2 -- hmp.c |2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/cpus.c b/cpus.c index 750a76f..cbc8556 100644 --- a/cpus.c +++ b/cpus.c @@ -1216,8 +1216,6 @@ CpuInfoList *qmp_query_cpus(Error **errp) info-value-has_nip = true; info-value-nip = env-nip; #elif defined(TARGET_SPARC) -info-value-has_pc = true; -info-value-pc = env-pc; info-value-has_npc = true; info-value-npc = env-npc; #elif defined(TARGET_MIPS) diff --git a/hmp.c b/hmp.c index 70bdec2..64b6ab5 100644 --- a/hmp.c +++ b/hmp.c @@ -242,7 +242,7 @@ void hmp_info_cpus(Monitor *mon) monitor_printf(mon, nip=0x%016 PRIx64, cpu-value-nip); } if (cpu-value-has_npc) { -monitor_printf(mon, pc=0x%016 PRIx64, cpu-value-pc); +monitor_printf(mon, pc=0x%016 PRIx64, cpu-value-pc); This patch changes qmp_query_cpus() to never set pc, so why do you print it? Well the print part of the code isn't change beside adding a space at the end. With the current code, has_npc implies has_pc, that's why I have removed the latter from qmp_query_cpus() in the sparc case. My point is that you dropped the pc assignment along: -info-value-pc = env-pc; So, value-pc will be zeroed. If you prefer to change this behavior and not have has_npc implying has_pc, I can send another patch. If pc is going to contain relevant info, then we have to set has_pc even if has_npc implies it. If your point is that pc is irrelevant, then we have to set has_pc to false and drop its printing from HMP. monitor_printf(mon, npc=0x%016 PRIx64, cpu-value-npc); } if (cpu-value-has_PC) {
Re: [Qemu-devel] [PATCH 01/18] error: add error_set_errno and error_setg_errno
Paolo Bonzini pbonz...@redhat.com writes: These functions help maintaining homogeneous formatting of error messages that include strerror values. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- error.c | 28 error.h | 9 + 2 file modificati, 37 inserzioni(+) diff --git a/error.c b/error.c index 1f05fc4..128d88c 100644 --- a/error.c +++ b/error.c @@ -43,6 +43,34 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) *errp = err; } +void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, + const char *fmt, ...) +{ +Error *err; +char *msg1; +va_list ap; + +if (errp == NULL) { +return; +} +assert(*errp == NULL); + +err = g_malloc0(sizeof(*err)); + +va_start(ap, fmt); +msg1 = g_strdup_vprintf(fmt, ap); +if (os_errno != 0) { +err-msg = g_strdup_printf(%s: %s, msg1, strerror(os_errno)); +g_free(msg1); +} else { +err-msg = msg1; +} +va_end(ap); +err-err_class = err_class; + +*errp = err; +} + Duplicates error_set() code without need. How about: void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, const char *fmt, ...) { char *msg; va_start(ap, fmt); msg = g_strdup_printf(fmt, ap); va_end(ap); if (os_errno) { error_set(errp, err_class, %s: %s, msg, strerror(os_errno)); } else { error_set(errp, err_class, %s, msg); } g_free(msg); } Sketch, not even compile-tested. Or the other way round, implement error_set() in terms of the more general error_set_errno(): void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) { error_set(errp, err_class, 0, fmt, ...); } Except that's not C; real code needs verror_set_errno(). As usual, every variadic function sooner or later needs a buddy that takes a va_list instead. Error *error_copy(const Error *err) { Error *err_new; [...]
Re: [Qemu-devel] [PATCH 01/18] error: add error_set_errno and error_setg_errno
Il 17/10/2012 14:56, Markus Armbruster ha scritto: Duplicates error_set() code without need. How about: void error_set_errno(Error **errp, int os_errno, ErrorClass err_class, const char *fmt, ...) { char *msg; va_start(ap, fmt); msg = g_strdup_printf(fmt, ap); va_end(ap); if (os_errno) { error_set(errp, err_class, %s: %s, msg, strerror(os_errno)); } else { error_set(errp, err_class, %s, msg); } g_free(msg); } Sketch, not even compile-tested. Or the other way round, implement error_set() in terms of the more general error_set_errno(): void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) { error_set(errp, err_class, 0, fmt, ...); } Except that's not C; real code needs verror_set_errno(). As usual, every variadic function sooner or later needs a buddy that takes a va_list instead. Indeed... lazy me. Added to my todo list. Paolo
Re: [Qemu-devel] [PATCH 02/18] qemu-sockets: add Error ** to all functions
Paolo Bonzini pbonz...@redhat.com writes: Il 04/10/2012 18:19, Luiz Capitulino ha scritto: -int unix_listen_opts(QemuOpts *opts) +int unix_listen_opts(QemuOpts *opts, Error **errp) { -fprintf(stderr, unix sockets are not available on windows\n); +error_setg(errp, unix sockets are not available on windows); Just gave me a review WTF, too. As we've discussed in a previous series, this breaks error reporting for unix_listen_opts() until errp is passed _and_ the error is routed to the right direction (in this case stderr). Is this fixed later in this series? If it's not, when do you plan to fix it? It is (which is why the single patch became 18), and I can move this hunk at the end of the series. Note that is just for Windows code and should really never run. Fair enough, but a note in the commit message is in order.
[Qemu-devel] [PATCH 2/4] Setting -sandbox on as deafult
Now the seccomp filter will be set to on even if no argument -sandbox is given. Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com --- configure | 2 +- vl.c | 38 +++--- 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/configure b/configure index 353d788..c613a51 100755 --- a/configure +++ b/configure @@ -220,7 +220,7 @@ guest_agent=yes want_tools=yes libiscsi= coroutine= -seccomp= +seccomp=yes glusterfs= # parse CC options first diff --git a/vl.c b/vl.c index 5b357a3..bec68cd 100644 --- a/vl.c +++ b/vl.c @@ -276,6 +276,10 @@ static int default_cdrom = 1; static int default_sdcard = 1; static int default_vga = 1; +#ifdef CONFIG_SECCOMP +bool seccomp_on = true; +#endif + static struct { const char *driver; int *flag; @@ -770,23 +774,28 @@ static int bt_parse(const char *opt) return 1; } -static int parse_sandbox(QemuOpts *opts, void *opaque) +static int install_seccomp_filters(void) { -/* FIXME: change this to true for 1.3 */ -if (qemu_opt_get_bool(opts, enable, false)) { #ifdef CONFIG_SECCOMP -if (seccomp_start() 0) { -qerror_report(ERROR_CLASS_GENERIC_ERROR, - failed to install seccomp syscall filter in the kernel); -return -1; -} -#else +if (seccomp_start() 0) { qerror_report(ERROR_CLASS_GENERIC_ERROR, - sandboxing request but seccomp is not compiled into this build); +failed to install seccomp syscall filter in the kernel); return -1; -#endif } +#else +qerror_report(ERROR_CLASS_GENERIC_ERROR, +sandboxing requested but seccomp is not compiled into this build); +return -1; +#endif +return 0; +} + +static int parse_sandbox(QemuOpts *opts, void *opaque) +{ +if (!qemu_opt_get_bool(opts, enable, true)) { +seccomp_on = false; +} return 0; } @@ -3320,6 +3329,13 @@ int main(int argc, char **argv, char **envp) exit(1); } +/* We should install seccomp filters even if -sandbox on is not used. */ +if (seccomp_on) { +if (install_seccomp_filters() 0) { +exit(1); +} +} + if (machine == NULL) { fprintf(stderr, No machine found.\n); exit(1); -- 1.7.12
[Qemu-devel] [PATCH 3/4] Support for double whitelist filters
This patch includes a second whitelist right before the main loop. It's a smaller and more restricted whitelist, excluding execve() among many others. Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com --- qemu-seccomp.c | 94 -- qemu-seccomp.h | 7 - vl.c | 13 +++- 3 files changed, 103 insertions(+), 11 deletions(-) diff --git a/qemu-seccomp.c b/qemu-seccomp.c index a25f2fa..9c68af5 100644 --- a/qemu-seccomp.c +++ b/qemu-seccomp.c @@ -13,6 +13,7 @@ * GNU GPL, version 2 or (at your option) any later version. */ #include stdio.h +#include stdlib.h #include seccomp.h #include qemu-seccomp.h @@ -21,7 +22,7 @@ struct QemuSeccompSyscall { uint8_t priority; }; -static const struct QemuSeccompSyscall seccomp_whitelist[] = { +static const struct QemuSeccompSyscall seccomp_whitelist_init[] = { { SCMP_SYS(timer_settime), 255 }, { SCMP_SYS(timer_gettime), 254 }, { SCMP_SYS(futex), 253 }, @@ -118,27 +119,102 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = { { SCMP_SYS(accept4), 242 } }; -int seccomp_start(void) +static const struct QemuSeccompSyscall seccomp_whitelist_main_loop[] = { +{ SCMP_SYS(timer_settime), 255 }, +{ SCMP_SYS(timer_gettime), 254 }, +{ SCMP_SYS(futex), 253 }, +{ SCMP_SYS(select), 252 }, +{ SCMP_SYS(recvfrom), 251 }, +{ SCMP_SYS(sendto), 250 }, +{ SCMP_SYS(read), 249 }, +{ SCMP_SYS(brk), 248 }, +{ SCMP_SYS(mmap), 247 }, +#if defined(__i386__) +{ SCMP_SYS(fcntl64), 245 }, +{ SCMP_SYS(fstat64), 245 }, +{ SCMP_SYS(stat64), 245 }, +{ SCMP_SYS(getgid32), 245 }, +{ SCMP_SYS(getegid32), 245 }, +{ SCMP_SYS(getuid32), 245 }, +{ SCMP_SYS(geteuid32), 245 }, +{ SCMP_SYS(sigreturn), 245 }, +{ SCMP_SYS(_newselect), 245 }, +{ SCMP_SYS(_llseek), 245 }, +{ SCMP_SYS(mmap2), 245}, +{ SCMP_SYS(sigprocmask), 245 }, +#endif +{ SCMP_SYS(exit), 245 }, +{ SCMP_SYS(timer_delete), 245 }, +{ SCMP_SYS(exit_group), 245 }, +{ SCMP_SYS(rt_sigreturn), 245 }, +{ SCMP_SYS(madvise), 245 }, +{ SCMP_SYS(write), 244 }, +{ SCMP_SYS(fcntl), 243 }, +{ SCMP_SYS(tgkill), 242 }, +{ SCMP_SYS(rt_sigaction), 242 }, +{ SCMP_SYS(pipe2), 242 }, +{ SCMP_SYS(munmap), 242 }, +{ SCMP_SYS(mremap), 242 }, +{ SCMP_SYS(getsockname), 242 }, +{ SCMP_SYS(getpeername), 242 }, +{ SCMP_SYS(close), 242 }, +{ SCMP_SYS(accept4), 242 } +}; + +static int +process_whitelist(const struct QemuSeccompSyscall *whitelist, + unsigned int size, scmp_filter_ctx *ctx) { int rc = 0; + unsigned int i = 0; -scmp_filter_ctx ctx; + +for (i = 0; i size; i++) { +rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, whitelist[i].num, 0); +if (rc 0) { +return -1; +} + +rc = seccomp_syscall_priority(ctx, whitelist[i].num, + whitelist[i].priority); +if (rc 0) { +return -1; +} +} +return 0; +} + +int +seccomp_start(enum whitelist_mode mode, scmp_filter_ctx *ctx) +{ +int rc = 0; ctx = seccomp_init(SCMP_ACT_KILL); if (ctx == NULL) { +rc = -1; goto seccomp_return; } -for (i = 0; i ARRAY_SIZE(seccomp_whitelist); i++) { -rc = seccomp_rule_add(ctx, SCMP_ACT_ALLOW, seccomp_whitelist[i].num, 0); -if (rc 0) { +switch (mode) { +case INIT: +if (process_whitelist +(seccomp_whitelist_init, + ARRAY_SIZE(seccomp_whitelist_init), ctx) 0) { +rc = -1; goto seccomp_return; } -rc = seccomp_syscall_priority(ctx, seccomp_whitelist[i].num, - seccomp_whitelist[i].priority); -if (rc 0) { +break; +case MAIN_LOOP: +if (process_whitelist +(seccomp_whitelist_main_loop, + ARRAY_SIZE(seccomp_whitelist_main_loop), ctx) 0) { +rc = -1; goto seccomp_return; } +break; +default: +rc = -1; +goto seccomp_return; } rc = seccomp_load(ctx); diff --git a/qemu-seccomp.h b/qemu-seccomp.h index b2fc3f8..1c97978 100644 --- a/qemu-seccomp.h +++ b/qemu-seccomp.h @@ -18,5 +18,10 @@ #include seccomp.h #include osdep.h -int seccomp_start(void); +enum whitelist_mode { +INIT = 0, +MAIN_LOOP = 1, +}; + +int seccomp_start(enum whitelist_mode mode, scmp_filter_ctx *ctx); #endif diff --git a/vl.c b/vl.c index bec68cd..773d488 100644 --- a/vl.c +++ b/vl.c @@ -278,6 +278,7 @@ static int default_vga = 1; #ifdef CONFIG_SECCOMP bool seccomp_on = true; +scmp_filter_ctx ctx; #endif static struct { @@ -777,7 +778,7 @@ static int bt_parse(const char *opt) static int install_seccomp_filters(void) { #ifdef CONFIG_SECCOMP -if (seccomp_start() 0) { +if (seccomp_start(INIT, ctx) 0) {
[Qemu-devel] [PATCH 4/4] Warning messages on net devices hotplug
With the inclusion of the new double whitelist seccomp filter, Qemu won't be able to execve() in runtime, thus, no hotplug net devices allowed. Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com --- hmp.c | 6 ++ net.c | 13 + 2 files changed, 19 insertions(+) diff --git a/hmp.c b/hmp.c index 70bdec2..f258338 100644 --- a/hmp.c +++ b/hmp.c @@ -1091,6 +1091,12 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict) Error *err = NULL; QemuOpts *opts; +#ifdef CONFIG_SECCOMP +error_set(err, ERROR_CLASS_GENERIC_ERROR, +Cannot hotplug TAP device when -sandbox is in effect); +goto out; +#endif + opts = qemu_opts_from_qdict(qemu_find_opts(netdev), qdict, err); if (error_is_set(err)) { goto out; diff --git a/net.c b/net.c index ae4bc0d..a652ee9 100644 --- a/net.c +++ b/net.c @@ -752,6 +752,12 @@ void net_host_device_add(Monitor *mon, const QDict *qdict) Error *local_err = NULL; QemuOpts *opts; +#ifdef CONFIG_SECCOMP +error_set(local_err, ERROR_CLASS_GENERIC_ERROR, +Cannot hotplug TAP device when -sandbox is in effect); +goto out; +#endif + if (!net_host_check_device(device)) { monitor_printf(mon, invalid host network device %s\n, device); return; @@ -765,6 +771,7 @@ void net_host_device_add(Monitor *mon, const QDict *qdict) qemu_opt_set(opts, type, device); net_client_init(opts, 0, local_err); +out: if (error_is_set(local_err)) { qerror_report_err(local_err); error_free(local_err); @@ -800,6 +807,12 @@ int qmp_netdev_add(Monitor *mon, const QDict *qdict, QObject **ret) QemuOptsList *opts_list; QemuOpts *opts; +#ifdef CONFIG_SECCOMP +error_set(local_err, ERROR_CLASS_GENERIC_ERROR, +Cannot hotplug TAP device when -sandbox is in effect); +goto exit_err; +#endif + opts_list = qemu_find_opts_err(netdev, local_err); if (error_is_set(local_err)) { goto exit_err; -- 1.7.12
Re: [Qemu-devel] [PATCH 03/18] qemu-sockets: unix_listen and unix_connect are portable
Paolo Bonzini pbonz...@redhat.com writes: They are just wrappers and do not need a Win32-specific version. This gave me pause (it's my thick-headed day, apparently), until I realized that in addition to what the commit message says you also move related code. Please try harder to make review as easy as possible.
Re: [Qemu-devel] [PATCH 03/18] qemu-sockets: unix_listen and unix_connect are portable
Il 17/10/2012 15:17, Markus Armbruster ha scritto: They are just wrappers and do not need a Win32-specific version. This gave me pause (it's my thick-headed day, apparently), until I realized that in addition to what the commit message says you also move related code. Please try harder to make review as easy as possible. I don't, it's just git that decided to make the diff this way. :) Paolo
[Qemu-devel] [PATCH 1/4] Adding new syscalls (bugzilla 855162)
According to the bug 855162[0] - there's the need of adding new syscalls to the whitelist whenn using Qemu with Libvirt. [1] - https://bugzilla.redhat.com/show_bug.cgi?id=855162 Reported-by: Paul Moore pmo...@redhat.com Signed-off-by: Eduardo Otubo ot...@linux.vnet.ibm.com --- qemu-seccomp.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/qemu-seccomp.c b/qemu-seccomp.c index 64329a3..a25f2fa 100644 --- a/qemu-seccomp.c +++ b/qemu-seccomp.c @@ -45,6 +45,13 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = { { SCMP_SYS(access), 245 }, { SCMP_SYS(prctl), 245 }, { SCMP_SYS(signalfd), 245 }, +{ SCMP_SYS(getrlimit), 245 }, +{ SCMP_SYS(set_tid_address), 245 }, +{ SCMP_SYS(socketpair), 245 }, +{ SCMP_SYS(statfs), 245 }, +{ SCMP_SYS(unlink), 245 }, +{ SCMP_SYS(wait4), 245 }, +{ SCMP_SYS(getuid), 245 }, #if defined(__i386__) { SCMP_SYS(fcntl64), 245 }, { SCMP_SYS(fstat64), 245 }, @@ -107,7 +114,8 @@ static const struct QemuSeccompSyscall seccomp_whitelist[] = { { SCMP_SYS(getsockname), 242 }, { SCMP_SYS(getpeername), 242 }, { SCMP_SYS(fdatasync), 242 }, -{ SCMP_SYS(close), 242 } +{ SCMP_SYS(close), 242 }, +{ SCMP_SYS(accept4), 242 } }; int seccomp_start(void) -- 1.7.12
[Qemu-devel] [PATCH 09/14] vga: stop direct access to DisplaySurface fields.
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/vga.c |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/vga.c b/hw/vga.c index a0ba94d..4d34469 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -1718,8 +1718,13 @@ static void vga_draw_graphic(VGACommonState *s, int full_update) s-last_depth = depth; full_update = 1; } else if (is_buffer_shared(s-ds-surface) - (full_update || s-ds-surface-data != s-vram_ptr + (s-start_addr * 4))) { -s-ds-surface-data = s-vram_ptr + (s-start_addr * 4); + (full_update || ds_get_data(s-ds) != s-vram_ptr ++ (s-start_addr * 4))) { +qemu_free_displaysurface(s-ds); +s-ds-surface = qemu_create_displaysurface_from(disp_width, +height, depth, +s-line_offset, +s-vram_ptr + (s-start_addr * 4)); dpy_gfx_setdata(s-ds); } -- 1.7.1
[Qemu-devel] [PATCH 04/14] pixman: helper functions
Add some helper functions which will be put into use by following patches. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- Makefile.objs |1 + qemu-pixman.c | 60 + qemu-pixman.h | 32 ++ 3 files changed, 93 insertions(+), 0 deletions(-) create mode 100644 qemu-pixman.c create mode 100644 qemu-pixman.h diff --git a/Makefile.objs b/Makefile.objs index 74b3542..73dd3bc 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -64,6 +64,7 @@ common-obj-y = $(block-obj-y) blockdev.o block/ common-obj-y += net.o net/ common-obj-y += qom/ common-obj-y += readline.o console.o cursor.o +common-obj-y += qemu-pixman.o common-obj-y += $(oslib-obj-y) common-obj-$(CONFIG_WIN32) += os-win32.o common-obj-$(CONFIG_POSIX) += os-posix.o diff --git a/qemu-pixman.c b/qemu-pixman.c new file mode 100644 index 000..7547ed7 --- /dev/null +++ b/qemu-pixman.c @@ -0,0 +1,60 @@ +#include qemu-pixman.h + +int qemu_pixman_get_type(int rshift, int gshift, int bshift) +{ +int type = PIXMAN_TYPE_OTHER; + +if (rshift gshift gshift bshift) { +if (bshift == 0) { +type = PIXMAN_TYPE_ARGB; +} else { +#if PIXMAN_VERSION = PIXMAN_VERSION_ENCODE(0, 21, 8) +type = PIXMAN_TYPE_RGBA; +#endif +} +} else if (rshift gshift gshift bshift) { +if (rshift == 0) { +type = PIXMAN_TYPE_ABGR; +} else { +type = PIXMAN_TYPE_BGRA; +} +} +return type; +} + +pixman_format_code_t qemu_pixman_get_format(PixelFormat *pf) +{ +pixman_format_code_t format; +int type; + +type = qemu_pixman_get_type(pf-rshift, pf-gshift, pf-bshift); +format = PIXMAN_FORMAT(pf-bits_per_pixel, type, + pf-abits, pf-rbits, pf-gbits, pf-bbits); +if (!pixman_format_supported_source(format)) { +return 0; +} +return format; +} + +pixman_image_t *qemu_pixman_linebuf_create(pixman_format_code_t format, + int width) +{ +pixman_image_t *image = pixman_image_create_bits(format, width, 1, NULL, 0); +assert(image != NULL); +return image; +} + +void qemu_pixman_linebuf_fill(pixman_image_t *linebuf, pixman_image_t *fb, + int width, int y) +{ +pixman_image_composite(PIXMAN_OP_SRC, fb, NULL, linebuf, + 0, y, 0, 0, 0, 0, width, 1); +} + +void qemu_pixman_image_unref(pixman_image_t *image) +{ +if (image == NULL) { +return; +} +pixman_image_unref(image); +} diff --git a/qemu-pixman.h b/qemu-pixman.h new file mode 100644 index 000..7652c41 --- /dev/null +++ b/qemu-pixman.h @@ -0,0 +1,32 @@ +#ifndef QEMU_PIXMAN_H +#define QEMU_PIXMAN_H + +#include pixman.h + +#include console.h + +/* + * pixman image formats are defined to be native endian, + * that means host byte order on qemu. So we go define + * fixed formats here for cases where it is needed, like + * feeding libjpeg / libpng and writing screenshots. + */ + +#ifdef HOST_WORDS_BIGENDIAN +# define PIXMAN_BE_r8g8b8 PIXMAN_r8g8b8 +#else +# define PIXMAN_BE_r8g8b8 PIXMAN_b8g8r8 +#endif + +/* */ + +int qemu_pixman_get_type(int rshift, int gshift, int bshift); +pixman_format_code_t qemu_pixman_get_format(PixelFormat *pf); + +pixman_image_t *qemu_pixman_linebuf_create(pixman_format_code_t format, + int width); +void qemu_pixman_linebuf_fill(pixman_image_t *linebuf, pixman_image_t *fb, + int width, int y); +void qemu_pixman_image_unref(pixman_image_t *image); + +#endif /* QEMU_PIXMAN_H */ -- 1.7.1
[Qemu-devel] [PATCH 02/14] pixman: add submodule
Add pixman submodule as fallback for old distros. Picking version 0.18.4. This is shipped by rhel6 and also the minimum version needed by spice so this should serve well as baseline. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- .gitmodules |3 +++ pixman |1 + 2 files changed, 4 insertions(+), 0 deletions(-) create mode 16 pixman diff --git a/.gitmodules b/.gitmodules index eca876f..cfa2af9 100644 --- a/.gitmodules +++ b/.gitmodules @@ -19,3 +19,6 @@ [submodule roms/sgabios] path = roms/sgabios url = git://git.qemu.org/sgabios.git +[submodule pixman] + path = pixman + url = git://anongit.freedesktop.org/pixman diff --git a/pixman b/pixman new file mode 16 index 000..97336fa --- /dev/null +++ b/pixman @@ -0,0 +1 @@ +Subproject commit 97336fad32acf802003855cd8bd6477fa49a12e3 -- 1.7.1
[Qemu-devel] [PATCH 06/14] console: make qemu_alloc_display static
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- console.c | 48 console.h |2 -- 2 files changed, 24 insertions(+), 26 deletions(-) diff --git a/console.c b/console.c index 5e1c5f5..48d88e4 100644 --- a/console.c +++ b/console.c @@ -1294,30 +1294,8 @@ static QemuConsole *new_console(DisplayState *ds, console_type_t console_type) return s; } -DisplaySurface *qemu_create_displaysurface(DisplayState *ds, - int width, int height) -{ -DisplaySurface *surface = g_new0(DisplaySurface, 1); - -int linesize = width * 4; -qemu_alloc_display(surface, width, height, linesize, - qemu_default_pixelformat(32), 0); -return surface; -} - -DisplaySurface *qemu_resize_displaysurface(DisplayState *ds, - int width, int height) -{ -int linesize = width * 4; - -trace_displaysurface_resize(ds, ds-surface, width, height); -qemu_alloc_display(ds-surface, width, height, linesize, - qemu_default_pixelformat(32), 0); -return ds-surface; -} - -void qemu_alloc_display(DisplaySurface *surface, int width, int height, -int linesize, PixelFormat pf, int newflags) +static void qemu_alloc_display(DisplaySurface *surface, int width, int height, + int linesize, PixelFormat pf, int newflags) { surface-width = width; surface-height = height; @@ -1342,6 +1320,28 @@ void qemu_alloc_display(DisplaySurface *surface, int width, int height, #endif } +DisplaySurface *qemu_create_displaysurface(DisplayState *ds, + int width, int height) +{ +DisplaySurface *surface = g_new0(DisplaySurface, 1); + +int linesize = width * 4; +qemu_alloc_display(surface, width, height, linesize, + qemu_default_pixelformat(32), 0); +return surface; +} + +DisplaySurface *qemu_resize_displaysurface(DisplayState *ds, + int width, int height) +{ +int linesize = width * 4; + +trace_displaysurface_resize(ds, ds-surface, width, height); +qemu_alloc_display(ds-surface, width, height, linesize, + qemu_default_pixelformat(32), 0); +return ds-surface; +} + DisplaySurface *qemu_create_displaysurface_from(int width, int height, int bpp, int linesize, uint8_t *data) { diff --git a/console.h b/console.h index c546e98..96ef165 100644 --- a/console.h +++ b/console.h @@ -190,8 +190,6 @@ void register_displaystate(DisplayState *ds); DisplayState *get_displaystate(void); DisplaySurface* qemu_create_displaysurface_from(int width, int height, int bpp, int linesize, uint8_t *data); -void qemu_alloc_display(DisplaySurface *surface, int width, int height, -int linesize, PixelFormat pf, int newflags); PixelFormat qemu_different_endianness_pixelformat(int bpp); PixelFormat qemu_default_pixelformat(int bpp); -- 1.7.1
[Qemu-devel] [PATCH 12/14] pixman/vnc: remove rgb_prepare_row* functions
Let pixman do it instead. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/vnc-enc-tight.c | 84 ++- 1 files changed, 10 insertions(+), 74 deletions(-) diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c index 8013c5c..9fd2556 100644 --- a/ui/vnc-enc-tight.c +++ b/ui/vnc-enc-tight.c @@ -1145,74 +1145,6 @@ static int send_palette_rect(VncState *vs, int x, int y, return (bytes = 0); } -#if defined(CONFIG_VNC_JPEG) || defined(CONFIG_VNC_PNG) -static void rgb_prepare_row24(VncState *vs, uint8_t *dst, int x, int y, - int count) -{ -VncDisplay *vd = vs-vd; -uint32_t *fbptr; -uint32_t pix; - -fbptr = vnc_server_fb_ptr(vd, x, y); - -while (count--) { -pix = *fbptr++; -*dst++ = (uint8_t)(pix vs-ds-surface-pf.rshift); -*dst++ = (uint8_t)(pix vs-ds-surface-pf.gshift); -*dst++ = (uint8_t)(pix vs-ds-surface-pf.bshift); -} -} - -#define DEFINE_RGB_GET_ROW_FUNCTION(bpp)\ -\ -static void \ -rgb_prepare_row##bpp(VncState *vs, uint8_t *dst,\ - int x, int y, int count) \ -{ \ -VncDisplay *vd = vs-vd;\ -uint##bpp##_t *fbptr; \ -uint##bpp##_t pix; \ -int r, g, b;\ -\ -fbptr = vnc_server_fb_ptr(vd, x, y);\ -\ -while (count--) { \ -pix = *fbptr++; \ -\ -r = (int)((pix vs-ds-surface-pf.rshift) \ - vs-ds-surface-pf.rmax); \ -g = (int)((pix vs-ds-surface-pf.gshift) \ - vs-ds-surface-pf.gmax); \ -b = (int)((pix vs-ds-surface-pf.bshift) \ - vs-ds-surface-pf.bmax); \ -\ -*dst++ = (uint8_t)((r * 255 + vs-ds-surface-pf.rmax / 2) \ - / vs-ds-surface-pf.rmax); \ -*dst++ = (uint8_t)((g * 255 + vs-ds-surface-pf.gmax / 2) \ - / vs-ds-surface-pf.gmax); \ -*dst++ = (uint8_t)((b * 255 + vs-ds-surface-pf.bmax / 2) \ - / vs-ds-surface-pf.bmax); \ -} \ -} - -DEFINE_RGB_GET_ROW_FUNCTION(16) -DEFINE_RGB_GET_ROW_FUNCTION(32) - -static void rgb_prepare_row(VncState *vs, uint8_t *dst, int x, int y, -int count) -{ -if (VNC_SERVER_FB_BYTES == 4) { -if (1) { -rgb_prepare_row24(vs, dst, x, y, count); -} else { -rgb_prepare_row32(vs, dst, x, y, count); -} -} else { -rgb_prepare_row16(vs, dst, x, y, count); -} -} -#endif /* CONFIG_VNC_JPEG or CONFIG_VNC_PNG */ - /* * JPEG compression stuff. */ @@ -1257,6 +1189,7 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality) struct jpeg_compress_struct cinfo; struct jpeg_error_mgr jerr; struct jpeg_destination_mgr manager; +pixman_image_t *linebuf; JSAMPROW row[1]; uint8_t *buf; int dy; @@ -1285,13 +1218,14 @@ static int send_jpeg_rect(VncState *vs, int x, int y, int w, int h, int quality) jpeg_start_compress(cinfo, true); -buf = g_malloc(w * 3); +linebuf = qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, w); +buf = (uint8_t *)pixman_image_get_data(linebuf); row[0] = buf; for (dy = 0; dy h; dy++) { -rgb_prepare_row(vs, buf, x, y + dy, w); +qemu_pixman_linebuf_fill(linebuf, vs-vd-server, w, dy); jpeg_write_scanlines(cinfo, row, 1); } -g_free(buf); +qemu_pixman_image_unref(linebuf); jpeg_finish_compress(cinfo); jpeg_destroy_compress(cinfo); @@ -1370,6 +1304,7 @@ static int send_png_rect(VncState *vs, int x, int y, int w, int h, png_structp png_ptr; png_infop info_ptr; png_colorp png_palette = NULL; +pixman_image_t *linebuf; int level = tight_png_conf[vs-tight.compression].png_zlib_level; int filters =
[Qemu-devel] [PATCH 14/14] pixman: drop obsolete fields from DisplaySurface
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- console.c |9 - console.h | 23 +-- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/console.c b/console.c index d28b75e..048b48e 100644 --- a/console.c +++ b/console.c @@ -1297,14 +1297,10 @@ static QemuConsole *new_console(DisplayState *ds, console_type_t console_type) static void qemu_alloc_display(DisplaySurface *surface, int width, int height, int linesize, PixelFormat pf, int newflags) { -surface-width = width; -surface-height = height; -surface-linesize = linesize; surface-pf = pf; qemu_pixman_image_unref(surface-image); surface-image = NULL; -surface-data = NULL; surface-format = qemu_pixman_get_format(pf); assert(surface-format != 0); @@ -1313,7 +1309,6 @@ static void qemu_alloc_display(DisplaySurface *surface, int width, int height, NULL, linesize); assert(surface-image != NULL); -surface-data = (uint8_t *)pixman_image_get_data(surface-image); surface-flags = newflags | QEMU_ALLOCATED_FLAG; #ifdef HOST_WORDS_BIGENDIAN surface-flags |= QEMU_BIG_ENDIAN_FLAG; @@ -1347,9 +1342,6 @@ DisplaySurface *qemu_create_displaysurface_from(int width, int height, int bpp, { DisplaySurface *surface = g_new0(DisplaySurface, 1); -surface-width = width; -surface-height = height; -surface-linesize = linesize; surface-pf = qemu_default_pixelformat(bpp); surface-format = qemu_pixman_get_format(surface-pf); @@ -1362,7 +1354,6 @@ DisplaySurface *qemu_create_displaysurface_from(int width, int height, int bpp, #ifdef HOST_WORDS_BIGENDIAN surface-flags = QEMU_BIG_ENDIAN_FLAG; #endif -surface-data = data; return surface; } diff --git a/console.h b/console.h index 96ef165..f66fbfc 100644 --- a/console.h +++ b/console.h @@ -123,10 +123,6 @@ struct DisplaySurface { pixman_format_code_t format; pixman_image_t *image; uint8_t flags; -int width; -int height; -int linesize;/* bytes per line */ -uint8_t *data; struct PixelFormat pf; }; @@ -346,32 +342,39 @@ static inline bool dpy_cursor_define_supported(struct DisplayState *s) static inline int ds_get_linesize(DisplayState *ds) { -return ds-surface-linesize; +return pixman_image_get_stride(ds-surface-image); } static inline uint8_t* ds_get_data(DisplayState *ds) { -return ds-surface-data; +return (void *)pixman_image_get_data(ds-surface-image); } static inline int ds_get_width(DisplayState *ds) { -return ds-surface-width; +return pixman_image_get_width(ds-surface-image); } static inline int ds_get_height(DisplayState *ds) { -return ds-surface-height; +return pixman_image_get_height(ds-surface-image); } static inline int ds_get_bits_per_pixel(DisplayState *ds) { -return ds-surface-pf.bits_per_pixel; +int bits = PIXMAN_FORMAT_BPP(ds-surface-format); +return bits; } static inline int ds_get_bytes_per_pixel(DisplayState *ds) { -return ds-surface-pf.bytes_per_pixel; +int bits = PIXMAN_FORMAT_BPP(ds-surface-format); +return (bits + 7) / 8; +} + +static inline pixman_format_code_t ds_get_format(DisplayState *ds) +{ +return ds-surface-format; } #ifdef CONFIG_CURSES -- 1.7.1
Re: [Qemu-devel] [PATCH 04/25] qemu-sockets: add nonblocking connect for Unix sockets
Paolo Bonzini pbonz...@redhat.com writes: Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- v1-v2: fixed connect_state memory leaks qemu-char.c| 2 +- qemu-sockets.c | 77 -- qemu_socket.h | 6 - 3 file modificati, 70 inserzioni(+), 15 rimozioni(-) diff --git a/qemu-char.c b/qemu-char.c index 3cc6cb5..8ebd582 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2450,7 +2450,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts) if (is_listen) { fd = unix_listen_opts(opts, NULL); } else { -fd = unix_connect_opts(opts, NULL); +fd = unix_connect_opts(opts, NULL, NULL, NULL); } } else { if (is_listen) { diff --git a/qemu-sockets.c b/qemu-sockets.c index f7e67b6..6a49429 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -252,16 +252,19 @@ static void wait_for_connect(void *opaque) } /* try to connect to the next address on the list */ -while (s-current_addr-ai_next != NULL s-fd 0) { -s-current_addr = s-current_addr-ai_next; -s-fd = inet_connect_addr(s-current_addr, in_progress, s); -/* connect in progress */ -if (in_progress) { -return; +if (s-current_addr) { +while (s-current_addr-ai_next != NULL s-fd 0) { +s-current_addr = s-current_addr-ai_next; +s-fd = inet_connect_addr(s-current_addr, in_progress, s); +/* connect in progress */ +if (in_progress) { +return; +} } + +freeaddrinfo(s-addr_list); } -freeaddrinfo(s-addr_list); if (s-callback) { s-callback(s-fd, s-opaque); } @@ -701,11 +704,13 @@ err: return -1; } -int unix_connect_opts(QemuOpts *opts, Error **errp) +int unix_connect_opts(QemuOpts *opts, Error **errp, + NonBlockingConnectHandler *callback, void *opaque) { struct sockaddr_un un; const char *path = qemu_opt_get(opts, path); -int sock; +ConnectState *connect_state = NULL; +int sock, rc; if (NULL == path) { fprintf(stderr, unix connect: no path specified\n); @@ -717,16 +722,44 @@ int unix_connect_opts(QemuOpts *opts, Error **errp) perror(socket(unix)); return -1; } +if (callback != NULL) { +connect_state = g_malloc0(sizeof(*connect_state)); +connect_state-callback = callback; +connect_state-opaque = opaque; +socket_set_nonblock(sock); +} memset(un, 0, sizeof(un)); un.sun_family = AF_UNIX; snprintf(un.sun_path, sizeof(un.sun_path), %s, path); -if (connect(sock, (struct sockaddr*) un, sizeof(un)) 0) { + +/* connect to peer */ +do { +rc = 0; +if (connect(sock, (struct sockaddr *) un, sizeof(un)) 0) { +rc = -socket_error(); +} +} while (rc == -EINTR); Isn't this a silent EINTR bug fix? + +if (connect_state != NULL QEMU_SOCKET_RC_INPROGRESS(rc)) { +connect_state-fd = sock; +qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect, + connect_state); +return sock; +} else { +/* non blocking socket immediate success, call callback */ +if (callback != NULL) { +callback(sock, opaque); +} +} + +if (rc 0) { fprintf(stderr, connect(unix:%s): %s\n, path, strerror(errno)); close(sock); - return -1; +sock = -1; } +g_free(connect_state); return sock; } Unlike inet_connect_opts(), this one runs callback() even when connect() fails. Are you sure that's what you want? [...]
Re: [Qemu-devel] [PATCH 05/25] migration: avoid using error_is_set and thus relying on errp != NULL
Paolo Bonzini pbonz...@redhat.com writes: The migration code is using errp to detect internal errors, this means that it relies on errp being non-NULL. Impact?
Re: [Qemu-devel] [PATCH 05/25] migration: avoid using error_is_set and thus relying on errp != NULL
Il 17/10/2012 15:36, Markus Armbruster ha scritto: The migration code is using errp to detect internal errors, this means that it relies on errp being non-NULL. Impact? None because so far our only QMP client is HMP and never passes a NULL Error **. But if we had others, it would make sure migration can work with a NULL Error **. Paolo
Re: [Qemu-devel] Any alternative to kqemu ?
On Mon, Oct 15, 2012 at 11:03 AM, Paolo Bonzini pbonz...@redhat.com wrote: Il 14/10/2012 12:52, Timothy Madden ha scritto: Hello Is kqemu deprecated ? It is simply not supported anymore. Is there an alternative to it ? No. That is tough ... ! So my hardware is officially obsolete. I had to get away with those other emulators provided by commercial companies. VMWare said my hardware does not support virtualization (much like qemu), but VirtualBox worked surprisingly well and uses the CPU natively (no emulation, faster than my qemu without kvm). `qemu-system-i386 -net nic ...` keeps saying upon invocation that vlan0 is not connected to host network. My `vconfig add eth1` command completed successfully, and I can `ifconfig eth1.0`, although I see no IP address on the new interface, only the mac address. QEMU VLANs have nothing to do with Linux VLANs. Yes, that is confusing. :( If you install libvirt, you can use -net nic -net bridge,br=virbr0. Otherwise, you could configure a bridge yourself and just use -net nic -net bridge. It turns out I had to say: ifconfig eth1.0 up Which might be obvious for the people here, but is not that much obvious when you first hear about vlans ... Thank you for your responses, Timothy Madden
Re: [Qemu-devel] [PATCH] hw/qxl: guest bug on primary create with stride %4 != 0
On 10/15/12 14:54, Alon Levy wrote: Due to usage of pixman for rendering on all spice surfaces we have pixman's requirement that the stride be word aligned. A guest not honoring that can crash spice and qemu with it due to failure to create a surface (in spice-server). Avoid this early on in primary surface creation and offscreen surface creation. Patch added to spice patch queue. thanks, Gerd
Re: [Qemu-devel] [PATCH 04/25] qemu-sockets: add nonblocking connect for Unix sockets
Il 17/10/2012 15:33, Markus Armbruster ha scritto: Paolo Bonzini pbonz...@redhat.com writes: Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- v1-v2: fixed connect_state memory leaks qemu-char.c| 2 +- qemu-sockets.c | 77 -- qemu_socket.h | 6 - 3 file modificati, 70 inserzioni(+), 15 rimozioni(-) diff --git a/qemu-char.c b/qemu-char.c index 3cc6cb5..8ebd582 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2450,7 +2450,7 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts) if (is_listen) { fd = unix_listen_opts(opts, NULL); } else { -fd = unix_connect_opts(opts, NULL); +fd = unix_connect_opts(opts, NULL, NULL, NULL); } } else { if (is_listen) { diff --git a/qemu-sockets.c b/qemu-sockets.c index f7e67b6..6a49429 100644 --- a/qemu-sockets.c +++ b/qemu-sockets.c @@ -252,16 +252,19 @@ static void wait_for_connect(void *opaque) } /* try to connect to the next address on the list */ -while (s-current_addr-ai_next != NULL s-fd 0) { -s-current_addr = s-current_addr-ai_next; -s-fd = inet_connect_addr(s-current_addr, in_progress, s); -/* connect in progress */ -if (in_progress) { -return; +if (s-current_addr) { +while (s-current_addr-ai_next != NULL s-fd 0) { +s-current_addr = s-current_addr-ai_next; +s-fd = inet_connect_addr(s-current_addr, in_progress, s); +/* connect in progress */ +if (in_progress) { +return; +} } + +freeaddrinfo(s-addr_list); } -freeaddrinfo(s-addr_list); if (s-callback) { s-callback(s-fd, s-opaque); } @@ -701,11 +704,13 @@ err: return -1; } -int unix_connect_opts(QemuOpts *opts, Error **errp) +int unix_connect_opts(QemuOpts *opts, Error **errp, + NonBlockingConnectHandler *callback, void *opaque) { struct sockaddr_un un; const char *path = qemu_opt_get(opts, path); -int sock; +ConnectState *connect_state = NULL; +int sock, rc; if (NULL == path) { fprintf(stderr, unix connect: no path specified\n); @@ -717,16 +722,44 @@ int unix_connect_opts(QemuOpts *opts, Error **errp) perror(socket(unix)); return -1; } +if (callback != NULL) { +connect_state = g_malloc0(sizeof(*connect_state)); +connect_state-callback = callback; +connect_state-opaque = opaque; +socket_set_nonblock(sock); +} memset(un, 0, sizeof(un)); un.sun_family = AF_UNIX; snprintf(un.sun_path, sizeof(un.sun_path), %s, path); -if (connect(sock, (struct sockaddr*) un, sizeof(un)) 0) { + +/* connect to peer */ +do { +rc = 0; +if (connect(sock, (struct sockaddr *) un, sizeof(un)) 0) { +rc = -socket_error(); +} +} while (rc == -EINTR); Isn't this a silent EINTR bug fix? + +if (connect_state != NULL QEMU_SOCKET_RC_INPROGRESS(rc)) { +connect_state-fd = sock; +qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect, + connect_state); +return sock; +} else { +/* non blocking socket immediate success, call callback */ +if (callback != NULL) { +callback(sock, opaque); +} +} + +if (rc 0) { fprintf(stderr, connect(unix:%s): %s\n, path, strerror(errno)); close(sock); -return -1; +sock = -1; } +g_free(connect_state); return sock; } Unlike inet_connect_opts(), this one runs callback() even when connect() fails. Are you sure that's what you want? I think it's harmless, but it's worth harmonizing it. Paolo
Re: [Qemu-devel] [PATCH v3 2/4] monitor: Enable adding an inherited fd to an fd set
Am 16.10.2012 20:08, schrieb Corey Bryant: qmp_add_fd() gets an fd that was received over a socket with SCM_RIGHTS and adds it to an fd set. This patch adds support that will enable adding an fd that was inherited on the command line to an fd set. Note: All of the code added to monitor_fdset_add_fd(), with the exception of the error path for non-valid fdset-id, is code motion from qmp_add_fd(). Signed-off-by: Corey Bryant cor...@linux.vnet.ibm.com And the non-valid fdset-id error case leaks fd now, right? Kevin
Re: [Qemu-devel] Any alternative to kqemu ?
Is there an alternative to it ? No. That is tough ... ! So my hardware is officially obsolete. I had to get away with those other emulators provided by commercial companies. VMWare said my hardware does not support virtualization (much like qemu), but VirtualBox worked surprisingly well and uses the CPU natively (no emulation, faster than my qemu without kvm). IIRC, VirtualBox take approach similar to kqemu does, i.e., translate privilege instructions only and run non-privilege instructions on host cpu directly. Regards, chenwj -- Wei-Ren Chen (陳韋任) Computer Systems Lab, Institute of Information Science, Academia Sinica, Taiwan (R.O.C.) Tel:886-2-2788-3799 #1667 Homepage: http://people.cs.nctu.edu.tw/~chenwj
Re: [Qemu-devel] Any alternative to kqemu ?
Il 17/10/2012 15:39, Timothy Madden ha scritto: On Mon, Oct 15, 2012 at 11:03 AM, Paolo Bonzini pbonz...@redhat.com wrote: Il 14/10/2012 12:52, Timothy Madden ha scritto: Hello Is kqemu deprecated ? It is simply not supported anymore. Is there an alternative to it ? No. That is tough ... ! So my hardware is officially obsolete. That is not because of obsoletion of hardware. It's because kqemu was holding off other enhancements to QEMU, and nobody stepped up to maintain it. I had to get away with those other emulators provided by commercial companies. VMWare said my hardware does not support virtualization (much like qemu), but VirtualBox worked surprisingly well and uses the CPU natively (no emulation, faster than my qemu without kvm). Yes, qemu without kvm can be quite slow because of the overhead of CPU emulation. One problem is that newer distros use SSE a lot, and this is quite slow. Paolo
Re: [Qemu-devel] nvram and boot order
On Wed, Oct 17, 2012 at 07:07:53AM +1100, Benjamin Herrenschmidt wrote: On Tue, 2012-10-16 at 14:55 -0500, Anthony Liguori wrote: 4) If -boot is specified, the parameter should alter the contents of NVRAM to change the boot order to what is specified by -boot. 5) If ,bootorder is specified, it should take predence over -boot. 6) ,bootorder= should also alter the contents of NVRAM to determine the boot order That's where I disagree. At least for us... I don't see why -boot or -bootorder should alter the nvram content. The plan is to have the nvram content essentially in control of SLOF (ie. the BIOS in x86 land). Qemu doesn't know much of anything appart from providing the storage for it. I see -boot and -bootorder as ways to *temporarily* override whatever setting was put in nvram by the guest. The nvram is typically populated by the distro installer (though sometimes by hand by the user). One may want to just temporarily boot from a CD (rescue for example, or to test a live CD or something ...), that doesn't mean the permanent setting should be altered. I would pass -boot and -bootorder to SLOF like we pass the current bootlist today and let it deal with it, I wouldn't touch the nvram. Agree with you and David. This is not QEMU busyness to touch nvram content. For all we know it may be encrypted by a key knows only to the firmware. -- Gleb.
Re: [Qemu-devel] [Bug 1066055] Re: Network performance regression with vde_switch
On Tue, Oct 16, 2012 at 2:23 PM, Edivaldo de Araujo Pereira edivaldoapere...@yahoo.com.br wrote: Hi Stefan I finally could revert that commit in the latest snapshot; problem was I needed to revert one later, that modified hw/virtio-serial-bus.c accordingly; after that reversion, the regression in network performance went completely away; this confirms my previous identification of the commit that caused it. Additionally, I tested your last suggestion, to change '' to '=', and that didn't help; the problem was still there. By the way, the performance gain I observed ,of about 25~30 % when using a tun/tap, was in fact just apparent, because it was result of a greater use of cpu, so it was achieved only when the host was idle; when I stress the host, with a lot of concurrent guests generating traffic, there is no gain at all. Just for confirmation, this is the performance I get with latest snapshot (8b4a3df8081f3e6f1061ed5cbb303ad623ade66b) running wget in the guest: $ wget -O /dev/null http://172.18.1.1/bpd.iso --2012-10-16 09:10:18-- http://172.18.1.1/bpd.iso Connecting to 172.18.1.1:80... connected. HTTP request sent, awaiting response... 200 OK Length: 358979584 (342M) [application/x-iso9660-image] Saving to: `/dev/null' 100%[==] 358.979.584 29,7M/s in 11s 2012-10-16 09:10:29 (30,3 MB/s) - `/dev/null' saved [358979584/358979584] The same wget, using the same snapshot, but with that commit reverted is: $ wget -O /dev/null http://172.18.1.1/bpd.iso --2012-10-16 09:15:12-- http://172.18.1.1/bpd.iso Connecting to 172.18.1.1:80... connected. HTTP request sent, awaiting response... 200 OK Length: 358979584 (342M) [application/x-iso9660-image] Saving to: `/dev/null' 100%[==] 358.979.584 180M/s in 1,9s 2012-10-16 09:15:14 (180 MB/s) - `/dev/null' saved [358979584/358979584] So, as I can see, there is no doubt: that commit is the culprit; as it was intended to be related just to the quality of the source code (at least as I can see), but implied in such a cost in performance, I think it would be better to revert it. Hi Amit, Edivaldo has identified the following commit responsible for a network performance regression he sees: commit 0d8d7690850eb0cf2b2b60933cf47669a6b6f18f Author: Amit Shah amit.s...@redhat.com Date: Tue Sep 25 00:05:15 2012 +0530 virtio: Introduce virtqueue_get_avail_bytes() I guess this is because we now iterate the entire descriptor chain to check available space instead of returning early. Do you want to propose a patch to fix it? Stefan
Re: [Qemu-devel] [PATCH v3 4/4] qemu-config: Add new -add-fd command line option
Am 17.10.2012 06:16, schrieb Eric Blake: I'm still seeing the corner case of: qemu-kvm -add-fd fd=3,set=1 -add-fd fd=4,set=2 4- where the dup(3) will populate fd 4 prior to the point where we get to process the -add-fd fd=4 command to notice that the user started qemu-kvm with fd 4 closed, and thus qemu will silently proceed to use the wrong fd. On the other hand, I'm not sure if that corner case is worth worrying about, or if we just chalk it up to user stupidity (aka libvirt programmer stupidity) if they did something like that (most likely, because the management app forgot to clear FD_CLOEXEC before exec()ing qemu-kvm). If you specify an FD number that isn't actually open when qemu is stared, you can get any FD that qemu opens internally. I think the correct answer to this problem is then don't do that. Hmm, this makes me wonder if I can do something crazy like: qemu-kvm -add-fd fd=4,set=1 -qmp /dev/fdset/1 to open a monitor on the fd I just passed in? I think so. At least on my side it was intended to allow this. And what if so, what then happens on that monitor if I request that fdset 1 be removed? The same as with block devices: The fd stays open until the monitor connection is closed. A closed monitor also triggers fd garbage collection, so at this point the original fd would be closed (well, assuming that you had only one monitor). Kevin
[Qemu-devel] [PATCH 13/14] pixman/vnc: remove dead code.
Switching the vnc server framebuffer to use 32bpp unconditionally turns the code bits which handle 8 and 16 bpp into dead code. Remove them. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/vnc-enc-hextile.c | 32 - ui/vnc-enc-tight.c | 64 +- ui/vnc.c | 18 -- 3 files changed, 27 insertions(+), 87 deletions(-) diff --git a/ui/vnc-enc-hextile.c b/ui/vnc-enc-hextile.c index 263a0ce..2e768fd 100644 --- a/ui/vnc-enc-hextile.c +++ b/ui/vnc-enc-hextile.c @@ -32,31 +32,11 @@ static void hextile_enc_cord(uint8_t *ptr, int x, int y, int w, int h) ptr[1] = (((w - 1) 0x0F) 4) | ((h - 1) 0x0F); } -#define BPP 8 -#include vnc-enc-hextile-template.h -#undef BPP - -#define BPP 16 -#include vnc-enc-hextile-template.h -#undef BPP - #define BPP 32 #include vnc-enc-hextile-template.h #undef BPP #define GENERIC -#define BPP 8 -#include vnc-enc-hextile-template.h -#undef BPP -#undef GENERIC - -#define GENERIC -#define BPP 16 -#include vnc-enc-hextile-template.h -#undef BPP -#undef GENERIC - -#define GENERIC #define BPP 32 #include vnc-enc-hextile-template.h #undef BPP @@ -89,24 +69,12 @@ void vnc_hextile_set_pixel_conversion(VncState *vs, int generic) { if (!generic) { switch (VNC_SERVER_FB_BITS) { -case 8: -vs-hextile.send_tile = send_hextile_tile_8; -break; -case 16: -vs-hextile.send_tile = send_hextile_tile_16; -break; case 32: vs-hextile.send_tile = send_hextile_tile_32; break; } } else { switch (VNC_SERVER_FB_BITS) { -case 8: -vs-hextile.send_tile = send_hextile_tile_generic_8; -break; -case 16: -vs-hextile.send_tile = send_hextile_tile_generic_16; -break; case 32: vs-hextile.send_tile = send_hextile_tile_generic_32; break; diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c index 9fd2556..9ae4cab 100644 --- a/ui/vnc-enc-tight.c +++ b/ui/vnc-enc-tight.c @@ -671,41 +671,35 @@ DEFINE_GRADIENT_FILTER_FUNCTION(32) * that case new color will be stored in *colorPtr. */ -#define DEFINE_CHECK_SOLID_FUNCTION(bpp)\ -\ -static bool \ -check_solid_tile##bpp(VncState *vs, int x, int y, int w, int h, \ - uint32_t* color, bool samecolor) \ -{ \ -VncDisplay *vd = vs-vd;\ -uint##bpp##_t *fbptr; \ -uint##bpp##_t c;\ -int dx, dy; \ -\ -fbptr = vnc_server_fb_ptr(vd, x, y);\ -\ -c = *fbptr; \ -if (samecolor (uint32_t)c != *color) { \ -return false; \ -} \ -\ -for (dy = 0; dy h; dy++) {\ -for (dx = 0; dx w; dx++) {\ -if (c != fbptr[dx]) { \ -return false; \ -} \ -} \ -fbptr = (uint##bpp##_t *) \ -((uint8_t *)fbptr + vnc_server_fb_stride(vd)); \ -} \ -\ -*color = (uint32_t)c; \ -return true;\ +static bool +check_solid_tile32(VncState *vs, int x, int y, int w, int h, + uint32_t *color, bool samecolor) +{ +VncDisplay *vd = vs-vd; +uint32_t *fbptr; +uint32_t c; +int dx, dy; + +fbptr = vnc_server_fb_ptr(vd, x, y); + +c = *fbptr; +if (samecolor (uint32_t)c != *color) { +return false; } -DEFINE_CHECK_SOLID_FUNCTION(32) -DEFINE_CHECK_SOLID_FUNCTION(16) -DEFINE_CHECK_SOLID_FUNCTION(8) +for (dy = 0; dy h; dy++) { +
[Qemu-devel] [PATCH 03/14] pixman: windup in configure makefiles
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- Makefile |9 + configure | 38 ++ 2 files changed, 47 insertions(+), 0 deletions(-) diff --git a/Makefile b/Makefile index a9c22bf..5699101 100644 --- a/Makefile +++ b/Makefile @@ -105,6 +105,15 @@ endif subdir-libcacard: $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o +subdir-pixman: pixman/Makefile + $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pixman V=$(V) all,) + +pixman/Makefile: $(SRC_PATH)/pixman/configure + (cd pixman; $(SRC_PATH)/pixman/configure --disable-shared --enable-static) + +$(SRC_PATH)/pixman/configure: + (cd $(SRC_PATH)/pixman; autoreconf -v --install) + $(filter %-softmmu,$(SUBDIR_RULES)): $(universal-obj-y) $(trace-obj-y) $(common-obj-y) $(extra-obj-y) subdir-libdis $(filter %-user,$(SUBDIR_RULES)): $(universal-obj-y) $(trace-obj-y) subdir-libdis-user subdir-libuser diff --git a/configure b/configure index 353d788..4b916aa 100755 --- a/configure +++ b/configure @@ -147,6 +147,7 @@ curses= docs= fdt= nptl= +pixman= sdl= virtfs= vnc=yes @@ -642,6 +643,10 @@ for opt do # configure to be used by RPM and similar macros that set # lots of directory switches by default. ;; + --pixman-system) pixman=system + ;; + --pixman-internal) pixman=internal + ;; --disable-sdl) sdl=no ;; --enable-sdl) sdl=yes @@ -2117,6 +2122,34 @@ else fi ## +# pixman support probe + +if test $pixman = ; then + if $pkg_config pixman-1 /dev/null 21; then +pixman=system + else +pixman=internal + fi +fi +if test $pixman = system; then + pixman_cflags=`$pkg_config --cflags pixman-1 2/dev/null` + pixman_libs=`$pkg_config --libs pixman-1 2/dev/null` +else + if test ! -d ${source_path}/pixman/pixman; then +echo ERROR: pixman not present. Your options: +echo (1) Prefered: Install the pixman devel package (any recent +echo distro should have packages as Xorg needs pixman too). +echo (2) Fetch the pixman submodule, using: +echo git submodule update --init pixman +exit 1 + fi + pixman_cflags=-I${source_path}/pixman/pixman + pixman_libs=-Lpixman/pixman/.libs -lpixman-1 +fi +QEMU_CFLAGS=$QEMU_CFLAGS $pixman_cflags +libs_softmmu=$libs_softmmu $pixman_libs + +## # libcap probe if test $cap != no ; then @@ -3147,6 +3180,7 @@ echo -Werror enabled $werror if test $darwin = yes ; then echo Cocoa support $cocoa fi +echo pixman$pixman echo SDL support $sdl echo curses support$curses echo curl support $curl @@ -3909,6 +3943,9 @@ if test $target_softmmu = yes ; then if test $smartcard_nss = yes ; then echo subdir-$target: subdir-libcacard $config_host_mak fi + if test $pixman = internal ; then +echo subdir-$target: subdir-pixman $config_host_mak + fi case $target_arch2 in i386|x86_64) echo CONFIG_HAVE_CORE_DUMP=y $config_target_mak @@ -4112,6 +4149,7 @@ DIRS=$DIRS pc-bios/optionrom pc-bios/spapr-rtas DIRS=$DIRS roms/seabios roms/vgabios DIRS=$DIRS qapi-generated DIRS=$DIRS libcacard libcacard/libcacard libcacard/trace +DIRS=$DIRS pixman FILES=Makefile tests/tcg/Makefile qdict-test-data.txt FILES=$FILES tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit FILES=$FILES tests/tcg/lm32/Makefile libcacard/Makefile -- 1.7.1
[Qemu-devel] [PATCH 08/14] qxl: stop direct access to DisplaySurface fields.
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/qxl-render.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/hw/qxl-render.c b/hw/qxl-render.c index 47eb8b4..98ecb21 100644 --- a/hw/qxl-render.c +++ b/hw/qxl-render.c @@ -24,7 +24,7 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect) { uint8_t *src; -uint8_t *dst = qxl-vga.ds-surface-data; +uint8_t *dst = ds_get_data(qxl-vga.ds); int len, i; if (is_buffer_shared(qxl-vga.ds-surface)) { -- 1.7.1
[Qemu-devel] [PATCHv2 1/2] signal: added a wrapper for sigprocmask function
Create a wrapper for signal mask changes initiated by the guest; this will give us a place to put code which prevents the guest from changing the handling of signals used by QEMU itself internally. The wrapper is called from all the guest-initiated sigprocmask, but is not called from internal qemu sigprocmask calls. Signed-off-by: Alex Barcelo abarc...@ac.upc.edu --- linux-user/qemu.h|1 + linux-user/signal.c | 10 ++ linux-user/syscall.c | 14 +++--- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/linux-user/qemu.h b/linux-user/qemu.h index fc4cc00..e2dd6a6 100644 --- a/linux-user/qemu.h +++ b/linux-user/qemu.h @@ -237,6 +237,7 @@ int host_to_target_signal(int sig); long do_sigreturn(CPUArchState *env); long do_rt_sigreturn(CPUArchState *env); abi_long do_sigaltstack(abi_ulong uss_addr, abi_ulong uoss_addr, abi_ulong sp); +int do_sigprocmask(int how, const sigset_t *set, sigset_t *oldset); #ifdef TARGET_I386 /* vm86.c */ diff --git a/linux-user/signal.c b/linux-user/signal.c index 15bc4e8..3d25b7d 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -5461,6 +5461,16 @@ long do_rt_sigreturn(CPUArchState *env) #endif +/* Wrapper for sigprocmask function + * Emulates a sigprocmask in a safe way for the guest. Note that set and oldset + * are host signal set, not guest ones. This wraps the sigprocmask host calls + * that should be protected (calls originated from guest) + */ +int do_sigprocmask(int how, const sigset_t *set, sigset_t *oldset) +{ +return sigprocmask(how, set, oldset); +} + void process_pending_signals(CPUArchState *cpu_env) { int sig; diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 471d060..49c0beb 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -5875,7 +5875,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { sigset_t cur_set; abi_ulong target_set; -sigprocmask(0, NULL, cur_set); +do_sigprocmask(0, NULL, cur_set); host_to_target_old_sigset(target_set, cur_set); ret = target_set; } @@ -5886,10 +5886,10 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, { sigset_t set, oset, cur_set; abi_ulong target_set = arg1; -sigprocmask(0, NULL, cur_set); +do_sigprocmask(0, NULL, cur_set); target_to_host_old_sigset(set, target_set); sigorset(set, set, cur_set); -sigprocmask(SIG_SETMASK, set, oset); +do_sigprocmask(SIG_SETMASK, set, oset); host_to_target_old_sigset(target_set, oset); ret = target_set; } @@ -5920,7 +5920,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, mask = arg2; target_to_host_old_sigset(set, mask); -ret = get_errno(sigprocmask(how, set, oldset)); +ret = get_errno(do_sigprocmask(how, set, oldset)); if (!is_error(ret)) { host_to_target_old_sigset(mask, oldset); ret = mask; @@ -5954,7 +5954,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, how = 0; set_ptr = NULL; } -ret = get_errno(sigprocmask(how, set_ptr, oldset)); +ret = get_errno(do_sigprocmask(how, set_ptr, oldset)); if (!is_error(ret) arg3) { if (!(p = lock_user(VERIFY_WRITE, arg3, sizeof(target_sigset_t), 0))) goto efault; @@ -5994,7 +5994,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, how = 0; set_ptr = NULL; } -ret = get_errno(sigprocmask(how, set_ptr, oldset)); +ret = get_errno(do_sigprocmask(how, set_ptr, oldset)); if (!is_error(ret) arg3) { if (!(p = lock_user(VERIFY_WRITE, arg3, sizeof(target_sigset_t), 0))) goto efault; @@ -7870,7 +7870,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, } mask = arg2; target_to_host_old_sigset(set, mask); -sigprocmask(how, set, oldset); +do_sigprocmask(how, set, oldset); host_to_target_old_sigset(mask, oldset); ret = mask; } -- 1.7.5.4
[Qemu-devel] [PATCHv2 0/2] Preparing safe sigprocmask wrapper on qemu-user
qemu-user needs SIGSEGV (at least) for some internal use. If the guest application masks it and does unsafe sigprocmask, then the application crashes. Problems happen in applications with self-modifying code (who also change the signal mask). Other guest applications may have related problems if they use the SIGSEGV. A way to be more safe is adding a wrapper for all sigprocmask calls from the guest. The wrapper proposed here is quite simple, but the code can be improved, here I try to ensure that the wrapper is set up properly. Here, a test case where qemu-user goes wrong: #include stdio.h #include stdlib.h #include string.h #include sys/mman.h #include malloc.h #include signal.h unsigned char *testfun; int main ( void ) { unsigned int ra; testfun=memalign(getpagesize(),1024); // We block the SIGSEGV signal, used by qemu-user sigset_t set; sigemptyset(set); sigaddset(set, 11); sigprocmask(SIG_BLOCK, set, NULL); mprotect(testfun, 1024, PROT_READ|PROT_EXEC|PROT_WRITE); //400687: b8 0d 00 00 00 mov$0xd,%eax //40068d: c3 retq testfun[ 0]=0xb8; testfun[ 1]=0x0d; testfun[ 2]=0x00; testfun[ 3]=0x00; testfun[ 4]=0x00; testfun[ 5]=0xc3; printf ( 0x%02X\n, ((unsigned int (*)())testfun)() ); //400687: b8 20 00 00 00 mov$0x20,%eax //40068d: c3 retq // This self-modifying code will break because of the sigsegv signal block testfun[ 1]=0x20; printf ( 0x%02X\n, ((unsigned int (*)())testfun)() ); } On an i386 native host: 0x0D 0x20 On a non-patched qemu-i386: 0x0D Segmentation fault Alex Barcelo (2): signal: added a wrapper for sigprocmask function signal: sigsegv protection on do_sigprocmask linux-user/qemu.h|1 + linux-user/signal.c | 27 +++ linux-user/syscall.c | 14 +++--- 3 files changed, 35 insertions(+), 7 deletions(-) -- 1.7.5.4
[Qemu-devel] [PATCHv2 2/2] signal: sigsegv protection on do_sigprocmask
Create a safe wrapper by protecting the signal mask. Instead of doing a simple passthrough of the sigprocmask, the wrapper manipulates the signal mask in a safe way for the qemu internal. This is done by avoiding SIGSEGV bit mask manipulation from the guest. We also return the same bit on the SIGSEGV. This is not required for most applications, but if the application checks it, then it will see that somethings fishy about it (and, in fact, maybe it should). If we do not want the guest to be aware of those manipulations, then it should be implemented in another way, but this seems quite clean and consistent. The wrapper can be improved to add more features for better signal managing, but this seems enough for simple self-modifying code. Signed-off-by: Alex Barcelo abarc...@ac.upc.edu --- linux-user/signal.c | 19 ++- 1 files changed, 18 insertions(+), 1 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index 3d25b7d..b965d89 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -5468,7 +5468,24 @@ long do_rt_sigreturn(CPUArchState *env) */ int do_sigprocmask(int how, const sigset_t *set, sigset_t *oldset) { -return sigprocmask(how, set, oldset); +int ret; +sigset_t val; +sigset_t *temp; +if (set) { +val = *set; +temp = val; +sigdelset(temp, SIGSEGV); +} else { +temp = NULL; +} +ret = sigprocmask(how, temp, oldset); + +/* Force set state of SIGSEGV, may be best for some apps, maybe not so good + * This is not required for qemu to work */ +if (oldset) { +sigaddset(oldset, SIGSEGV); +} +return ret; } void process_pending_signals(CPUArchState *cpu_env) -- 1.7.5.4
[Qemu-devel] [PATCH 00/14] pixman patch series.
Hi, First round of pixman patches for qemu, so we have something concrete to discuss. Applies on top of the console cleanups patch series posted a few days ago. The long-term goal is to stop reinventing the wheel over and over when it comes to pixel manipulation and to use pixman instead. This patch series is a start. It switches a central data structure (DisplaySurface) over to pixman, so the ui code can easily use pixman to manipulate pixel data. It also collects some low-hanging fruit to show pixman in action. Comments? cheers, Gerd Gerd Hoffmann (14): console: remove DisplayAllocator pixman: add submodule pixman: windup in configure makefiles pixman: helper functions pixman: add pixman image to DisplaySurface console: make qemu_alloc_display static console: don't set PixelFormat alpha fields for 32bpp qxl: stop direct access to DisplaySurface fields. vga: stop direct access to DisplaySurface fields. pixman: switch screendump function. pixman/vnc: use pixman images in vnc. pixman/vnc: remove rgb_prepare_row* functions pixman/vnc: remove dead code. pixman: drop obsolete fields from DisplaySurface .gitmodules |3 + Makefile |9 ++ Makefile.objs |1 + configure | 38 ++ console.c | 107 +++- console.h | 62 -- hw/qxl-render.c |2 +- hw/vga.c | 48 +++- pixman|1 + qemu-common.h |1 - qemu-pixman.c | 60 + qemu-pixman.h | 32 + ui/sdl.c | 117 + ui/vnc-enc-hextile-template.h | 23 ++-- ui/vnc-enc-hextile.c | 53 ++--- ui/vnc-enc-tight.c| 280 +++-- ui/vnc-enc-zrle.c | 18 ++-- ui/vnc-jobs.c |3 +- ui/vnc.c | 239 ++- ui/vnc.h | 19 +++- 20 files changed, 508 insertions(+), 608 deletions(-) create mode 16 pixman create mode 100644 qemu-pixman.c create mode 100644 qemu-pixman.h
Re: [Qemu-devel] [PATCH 0/2] signal: Preparing safe sigprocmask wrapper on qemu-user
Thanks Peter for all the feedback! I have just sent the v2 (have been on holiday this weekend, a bit offline). I hope that this one is a better patch. Sorry Riku, I have doubleposted the new patch to you, I slipped on the git send-email command, and sent the patch only to you at first (not to the list). I corrected it but now you may have received twice. Next time I will double check my git output :( On Wed, Oct 10, 2012 at 5:37 PM, Peter Maydell peter.mayd...@linaro.org wrote: On 8 October 2012 19:42, Alex Barcelo abarc...@ac.upc.edu wrote: okay, now I see that this lacks a lot of presentation. Before sending a v2, is there something more that I should correct? Well, yes, your cover letter could be a little more verbose, but I think mostly it's just that nobody's got round to reviewing the patches yet. I'll have a look at them in a moment. One thing that is definitely missing and is critical is that you need to include a Signed-off-by: line in your patches' commit messages: we cannot commit them without one. (http://wiki.qemu.org/Contribute/SubmitAPatch mentions this and has some other hints, if you haven't read it.) And one netiquete question, test-breaking code should be in the description (cover, 00/00)? On a next post in the same thread? Another thread? In the cover letter is fine. -- PMM
Re: [Qemu-devel] [PATCH 03/14] pixman: windup in configure makefiles
Il 17/10/2012 15:29, Gerd Hoffmann ha scritto: Signed-off-by: Gerd Hoffmann kra...@redhat.com --- Makefile |9 + configure | 38 ++ 2 files changed, 47 insertions(+), 0 deletions(-) diff --git a/Makefile b/Makefile index a9c22bf..5699101 100644 --- a/Makefile +++ b/Makefile @@ -105,6 +105,15 @@ endif subdir-libcacard: $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o +subdir-pixman: pixman/Makefile + $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pixman V=$(V) all,) + +pixman/Makefile: $(SRC_PATH)/pixman/configure + (cd pixman; $(SRC_PATH)/pixman/configure --disable-shared --enable-static) + +$(SRC_PATH)/pixman/configure: + (cd $(SRC_PATH)/pixman; autoreconf -v --install) + $(filter %-softmmu,$(SUBDIR_RULES)): $(universal-obj-y) $(trace-obj-y) $(common-obj-y) $(extra-obj-y) subdir-libdis $(filter %-user,$(SUBDIR_RULES)): $(universal-obj-y) $(trace-obj-y) subdir-libdis-user subdir-libuser diff --git a/configure b/configure index 353d788..4b916aa 100755 --- a/configure +++ b/configure @@ -147,6 +147,7 @@ curses= docs= fdt= nptl= +pixman= sdl= virtfs= vnc=yes @@ -642,6 +643,10 @@ for opt do # configure to be used by RPM and similar macros that set # lots of directory switches by default. ;; + --pixman-system) pixman=system + ;; + --pixman-internal) pixman=internal + ;; Let's make this more autoconfy: --with-system-pixman/--without-system-pixman. Paolo --disable-sdl) sdl=no ;; --enable-sdl) sdl=yes @@ -2117,6 +2122,34 @@ else fi ## +# pixman support probe + +if test $pixman = ; then + if $pkg_config pixman-1 /dev/null 21; then +pixman=system + else +pixman=internal + fi +fi +if test $pixman = system; then + pixman_cflags=`$pkg_config --cflags pixman-1 2/dev/null` + pixman_libs=`$pkg_config --libs pixman-1 2/dev/null` +else + if test ! -d ${source_path}/pixman/pixman; then +echo ERROR: pixman not present. Your options: +echo (1) Prefered: Install the pixman devel package (any recent +echo distro should have packages as Xorg needs pixman too). +echo (2) Fetch the pixman submodule, using: +echo git submodule update --init pixman +exit 1 + fi + pixman_cflags=-I${source_path}/pixman/pixman + pixman_libs=-Lpixman/pixman/.libs -lpixman-1 +fi +QEMU_CFLAGS=$QEMU_CFLAGS $pixman_cflags +libs_softmmu=$libs_softmmu $pixman_libs + +## # libcap probe if test $cap != no ; then @@ -3147,6 +3180,7 @@ echo -Werror enabled $werror if test $darwin = yes ; then echo Cocoa support $cocoa fi +echo pixman$pixman echo SDL support $sdl echo curses support$curses echo curl support $curl @@ -3909,6 +3943,9 @@ if test $target_softmmu = yes ; then if test $smartcard_nss = yes ; then echo subdir-$target: subdir-libcacard $config_host_mak fi + if test $pixman = internal ; then +echo subdir-$target: subdir-pixman $config_host_mak + fi case $target_arch2 in i386|x86_64) echo CONFIG_HAVE_CORE_DUMP=y $config_target_mak @@ -4112,6 +4149,7 @@ DIRS=$DIRS pc-bios/optionrom pc-bios/spapr-rtas DIRS=$DIRS roms/seabios roms/vgabios DIRS=$DIRS qapi-generated DIRS=$DIRS libcacard libcacard/libcacard libcacard/trace +DIRS=$DIRS pixman FILES=Makefile tests/tcg/Makefile qdict-test-data.txt FILES=$FILES tests/tcg/cris/Makefile tests/tcg/cris/.gdbinit FILES=$FILES tests/tcg/lm32/Makefile libcacard/Makefile
Re: [Qemu-devel] [PATCH 07/25] migration: use qemu-sockets to establish Unix sockets
Paolo Bonzini pbonz...@redhat.com writes: This makes migration-unix.c again a cut-and-paste job from migration-tcp.c, exactly as it was in the beginning. :) Neat!
[Qemu-devel] [PATCH 01/14] console: remove DisplayAllocator
Causes [temporary] preformance regression with 24bpp vga modes @ sdl. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- console.c | 52 + console.h | 34 +++-- qemu-common.h |1 - ui/sdl.c | 117 +++-- 4 files changed, 31 insertions(+), 173 deletions(-) diff --git a/console.c b/console.c index 61812c7..71cc543 100644 --- a/console.c +++ b/console.c @@ -1294,9 +1294,10 @@ static QemuConsole *new_console(DisplayState *ds, console_type_t console_type) return s; } -static DisplaySurface* defaultallocator_create_displaysurface(int width, int height) +DisplaySurface *qemu_create_displaysurface(DisplayState *ds, + int width, int height) { -DisplaySurface *surface = (DisplaySurface*) g_malloc0(sizeof(DisplaySurface)); +DisplaySurface *surface = g_new0(DisplaySurface, 1); int linesize = width * 4; qemu_alloc_display(surface, width, height, linesize, @@ -1304,13 +1305,15 @@ static DisplaySurface* defaultallocator_create_displaysurface(int width, int hei return surface; } -static DisplaySurface* defaultallocator_resize_displaysurface(DisplaySurface *surface, - int width, int height) +DisplaySurface *qemu_resize_displaysurface(DisplayState *ds, + int width, int height) { int linesize = width * 4; -qemu_alloc_display(surface, width, height, linesize, + +trace_displaysurface_resize(ds, ds-surface, width, height); +qemu_alloc_display(ds-surface, width, height, linesize, qemu_default_pixelformat(32), 0); -return surface; +return ds-surface; } void qemu_alloc_display(DisplaySurface *surface, int width, int height, @@ -1323,7 +1326,7 @@ void qemu_alloc_display(DisplaySurface *surface, int width, int height, surface-pf = pf; if (surface-flags QEMU_ALLOCATED_FLAG) { data = g_realloc(surface-data, -surface-linesize * surface-height); + surface-linesize * surface-height); } else { data = g_malloc(surface-linesize * surface-height); } @@ -1334,7 +1337,7 @@ void qemu_alloc_display(DisplaySurface *surface, int width, int height, #endif } -DisplaySurface* qemu_create_displaysurface_from(int width, int height, int bpp, +DisplaySurface *qemu_create_displaysurface_from(int width, int height, int bpp, int linesize, uint8_t *data) { DisplaySurface *surface = (DisplaySurface*) g_malloc0(sizeof(DisplaySurface)); @@ -1351,28 +1354,24 @@ DisplaySurface* qemu_create_displaysurface_from(int width, int height, int bpp, return surface; } -static void defaultallocator_free_displaysurface(DisplaySurface *surface) +void qemu_free_displaysurface(DisplayState *ds) { -if (surface == NULL) +trace_displaysurface_free(ds, ds-surface); +if (ds-surface == NULL) { return; -if (surface-flags QEMU_ALLOCATED_FLAG) -g_free(surface-data); -g_free(surface); +} +if (ds-surface-flags QEMU_ALLOCATED_FLAG) { +g_free(ds-surface-data); +} +g_free(ds-surface); } -static struct DisplayAllocator default_allocator = { -defaultallocator_create_displaysurface, -defaultallocator_resize_displaysurface, -defaultallocator_free_displaysurface -}; - static void dumb_display_init(void) { DisplayState *ds = g_malloc0(sizeof(DisplayState)); int width = 640; int height = 480; -ds-allocator = default_allocator; if (is_fixedsize_console()) { width = active_console-g_width; height = active_console-g_height; @@ -1402,18 +1401,6 @@ DisplayState *get_displaystate(void) return display_state; } -DisplayAllocator *register_displayallocator(DisplayState *ds, DisplayAllocator *da) -{ -if(ds-allocator == default_allocator) { -DisplaySurface *surf; -surf = da-create_displaysurface(ds_get_width(ds), ds_get_height(ds)); -defaultallocator_free_displaysurface(ds-surface); -ds-surface = surf; -ds-allocator = da; -} -return ds-allocator; -} - DisplayState *graphic_console_init(vga_hw_update_ptr update, vga_hw_invalidate_ptr invalidate, vga_hw_screen_dump_ptr screen_dump, @@ -1424,7 +1411,6 @@ DisplayState *graphic_console_init(vga_hw_update_ptr update, DisplayState *ds; ds = (DisplayState *) g_malloc0(sizeof(DisplayState)); -ds-allocator = default_allocator; ds-surface = qemu_create_displaysurface(ds, 640, 480); s = new_console(ds, GRAPHIC_CONSOLE); diff --git a/console.h b/console.h index 78e842f..8f13668 100644 --- a/console.h +++ b/console.h @@ -107,7 +107,6 @@ void kbd_put_keysym(int keysym); #define QEMU_BIG_ENDIAN_FLAG
[Qemu-devel] [PATCH 10/14] pixman: switch screendump function.
Signed-off-by: Gerd Hoffmann kra...@redhat.com --- hw/vga.c | 39 +++ 1 files changed, 11 insertions(+), 28 deletions(-) diff --git a/hw/vga.c b/hw/vga.c index 4d34469..4ffd57c 100644 --- a/hw/vga.c +++ b/hw/vga.c @@ -2417,13 +2417,12 @@ void vga_init_vbe(VGACommonState *s, MemoryRegion *system_memory) void ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp) { +int width = pixman_image_get_width(ds-image); +int height = pixman_image_get_height(ds-image); FILE *f; -uint8_t *d, *d1; -uint32_t v; -int y, x; -uint8_t r, g, b; +int y; int ret; -char *linebuf, *pbuf; +pixman_image_t *linebuf; trace_ppm_save(filename, ds); f = fopen(filename, wb); @@ -2432,33 +2431,17 @@ void ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp) strerror(errno)); return; } -ret = fprintf(f, P6\n%d %d\n%d\n, ds-width, ds-height, 255); +ret = fprintf(f, P6\n%d %d\n%d\n, width, height, 255); if (ret 0) { linebuf = NULL; goto write_err; } -linebuf = g_malloc(ds-width * 3); -d1 = ds-data; -for(y = 0; y ds-height; y++) { -d = d1; -pbuf = linebuf; -for(x = 0; x ds-width; x++) { -if (ds-pf.bits_per_pixel == 32) -v = *(uint32_t *)d; -else -v = (uint32_t) (*(uint16_t *)d); -/* Limited to 8 or fewer bits per channel: */ -r = ((v ds-pf.rshift) ds-pf.rmax) (8 - ds-pf.rbits); -g = ((v ds-pf.gshift) ds-pf.gmax) (8 - ds-pf.gbits); -b = ((v ds-pf.bshift) ds-pf.bmax) (8 - ds-pf.bbits); -*pbuf++ = r; -*pbuf++ = g; -*pbuf++ = b; -d += ds-pf.bytes_per_pixel; -} -d1 += ds-linesize; +linebuf = qemu_pixman_linebuf_create(PIXMAN_BE_r8g8b8, width); +for (y = 0; y height; y++) { +qemu_pixman_linebuf_fill(linebuf, ds-image, width, y); clearerr(f); -ret = fwrite(linebuf, 1, pbuf - linebuf, f); +ret = fwrite(pixman_image_get_data(linebuf), 1, + pixman_image_get_stride(linebuf), f); (void)ret; if (ferror(f)) { goto write_err; @@ -2466,7 +2449,7 @@ void ppm_save(const char *filename, struct DisplaySurface *ds, Error **errp) } out: -g_free(linebuf); +qemu_pixman_image_unref(linebuf); fclose(f); return; -- 1.7.1
[Qemu-devel] [PATCH 07/14] console: don't set PixelFormat alpha fields for 32bpp
Currently it is inconstent, PixelFormat-amask is left unset whereas abits and amax and ashift are filled. As an alpha channel doesn't make sense for the vga framebuffer leave all alpha fields clear. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- console.c |3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/console.c b/console.c index 48d88e4..d28b75e 100644 --- a/console.c +++ b/console.c @@ -1715,18 +1715,15 @@ PixelFormat qemu_default_pixelformat(int bpp) pf.rmask = 0x00FF; pf.gmask = 0xFF00; pf.bmask = 0x00FF; -pf.amax = 255; pf.rmax = 255; pf.gmax = 255; pf.bmax = 255; -pf.ashift = 24; pf.rshift = 16; pf.gshift = 8; pf.bshift = 0; pf.rbits = 8; pf.gbits = 8; pf.bbits = 8; -pf.abits = 8; break; default: break; -- 1.7.1
[Qemu-devel] [PATCH 05/14] pixman: add pixman image to DisplaySurface
Surfaces are now allocated using pixman. DisplaySurface gets new struct fields with pixman image and data. DisplayChangeListeners can easily start using pixman now. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- console.c | 37 - console.h |3 +++ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/console.c b/console.c index 71cc543..5e1c5f5 100644 --- a/console.c +++ b/console.c @@ -1319,18 +1319,23 @@ DisplaySurface *qemu_resize_displaysurface(DisplayState *ds, void qemu_alloc_display(DisplaySurface *surface, int width, int height, int linesize, PixelFormat pf, int newflags) { -void *data; surface-width = width; surface-height = height; surface-linesize = linesize; surface-pf = pf; -if (surface-flags QEMU_ALLOCATED_FLAG) { -data = g_realloc(surface-data, - surface-linesize * surface-height); -} else { -data = g_malloc(surface-linesize * surface-height); -} -surface-data = (uint8_t *)data; + +qemu_pixman_image_unref(surface-image); +surface-image = NULL; +surface-data = NULL; + +surface-format = qemu_pixman_get_format(pf); +assert(surface-format != 0); +surface-image = pixman_image_create_bits(surface-format, + width, height, + NULL, linesize); +assert(surface-image != NULL); + +surface-data = (uint8_t *)pixman_image_get_data(surface-image); surface-flags = newflags | QEMU_ALLOCATED_FLAG; #ifdef HOST_WORDS_BIGENDIAN surface-flags |= QEMU_BIG_ENDIAN_FLAG; @@ -1338,14 +1343,22 @@ void qemu_alloc_display(DisplaySurface *surface, int width, int height, } DisplaySurface *qemu_create_displaysurface_from(int width, int height, int bpp, - int linesize, uint8_t *data) +int linesize, uint8_t *data) { -DisplaySurface *surface = (DisplaySurface*) g_malloc0(sizeof(DisplaySurface)); +DisplaySurface *surface = g_new0(DisplaySurface, 1); surface-width = width; surface-height = height; surface-linesize = linesize; surface-pf = qemu_default_pixelformat(bpp); + +surface-format = qemu_pixman_get_format(surface-pf); +assert(surface-format != 0); +surface-image = pixman_image_create_bits(surface-format, + width, height, + (void *)data, linesize); +assert(surface-image != NULL); + #ifdef HOST_WORDS_BIGENDIAN surface-flags = QEMU_BIG_ENDIAN_FLAG; #endif @@ -1360,9 +1373,7 @@ void qemu_free_displaysurface(DisplayState *ds) if (ds-surface == NULL) { return; } -if (ds-surface-flags QEMU_ALLOCATED_FLAG) { -g_free(ds-surface-data); -} +qemu_pixman_image_unref(ds-surface-image); g_free(ds-surface); } diff --git a/console.h b/console.h index 8f13668..c546e98 100644 --- a/console.h +++ b/console.h @@ -2,6 +2,7 @@ #define CONSOLE_H #include qemu-char.h +#include qemu-pixman.h #include qdict.h #include notify.h #include monitor.h @@ -119,6 +120,8 @@ struct PixelFormat { }; struct DisplaySurface { +pixman_format_code_t format; +pixman_image_t *image; uint8_t flags; int width; int height; -- 1.7.1
Re: [Qemu-devel] [PATCH 14/22] usb: Add packet combining functions
Hi, On 10/17/2012 01:29 PM, Gerd Hoffmann wrote: Hi, +/* + * Process / cancel combined packets, called from + * usb_ep_combine_input_packets() / usb_combined_packet_cancel(). + * Only called for devices which call these functions themselves. + */ +int (*handle_combined_data)(USBDevice *dev, USBPacket *p); +void (*cancel_combined_packet)(USBDevice *dev, USBPacket *p); Do we really need these? For handle_combined_data, yes, as usb_ep_combine_input_packets can cause multiple packets to get submitted, since if a combined packet ends with a packet, which does not have its short_not_ok flag set, another packet can be safely pipelined after it. This is not useful for usb mass storage, but very usefull for usb serial port converters. I actually first did not have cancel_combined_packet :) Instead I had usb_combined_packet_cancel() return a boolean indicating if it had handled the cancel, or if the caller (which would be a device's cancel method) needs to handle the cancel itself. I personally find the cancel_combined_packet callback somewhat cleaner, esp. since with the return boolean method, after calling usb_combined_packet_cancel(p) p-combined will be NULL, so if the device's cancel method needs access to p-combined to deal with combined packets, things get more difficult. But if you prefer getting rid of the callback we can do that. I think it isn't much work for the callers to do that themself. Saves them providing a callback. And makes the code flow easier to follow by removing a pointless indirection. For handle_combined_data we probably must make usb_ep_combine_input_packets return a status code. Regards, Hans
[Qemu-devel] [PATCH 11/14] pixman/vnc: use pixman images in vnc.
The vnc code uses *three* DisplaySurfaces: First is the surface of the actual QemuConsole, usually the guest screen, but could also be a text console (monitor/serial reachable via Ctrl-Alt-nr keys). This is left as-is. Second is the current server's view of the screen content. The vnc code uses this to figure which parts of the guest screen did _really_ change to reduce the amount of updates sent to the vnc clients. It is also used as data source when sending out the updates to the clients. This surface gets replaced by a pixman image. The format changes too, instead of using the guest screen format we'll use fixed 32bit rgb framebuffer and convert the pixels on the fly when comparing and updating the server framebuffer. Third surface carries the format expected by the vnc client. That isn't used to store image data. This surface is switched to PixelFormat and a boolean for bigendian byte order. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/vnc-enc-hextile-template.h | 23 ++-- ui/vnc-enc-hextile.c | 45 ui/vnc-enc-tight.c| 144 - ui/vnc-enc-zrle.c | 18 ++-- ui/vnc-jobs.c |3 +- ui/vnc.c | 235 +++-- ui/vnc.h | 19 +++- 7 files changed, 259 insertions(+), 228 deletions(-) diff --git a/ui/vnc-enc-hextile-template.h b/ui/vnc-enc-hextile-template.h index a7310e1..d868d75 100644 --- a/ui/vnc-enc-hextile-template.h +++ b/ui/vnc-enc-hextile-template.h @@ -14,7 +14,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, int *has_bg, int *has_fg) { VncDisplay *vd = vs-vd; -uint8_t *row = vd-server-data + y * ds_get_linesize(vs-ds) + x * ds_get_bytes_per_pixel(vs-ds); +uint8_t *row = vnc_server_fb_ptr(vd, x, y); pixel_t *irow = (pixel_t *)row; int j, i; pixel_t *last_bg = (pixel_t *)last_bg_; @@ -25,7 +25,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, int bg_count = 0; int fg_count = 0; int flags = 0; -uint8_t data[(vs-clientds.pf.bytes_per_pixel + 2) * 16 * 16]; +uint8_t data[(vs-client_pf.bytes_per_pixel + 2) * 16 * 16]; int n_data = 0; int n_subtiles = 0; @@ -58,7 +58,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, } if (n_colors 2) break; - irow += ds_get_linesize(vs-ds) / sizeof(pixel_t); + irow += vnc_server_fb_stride(vd) / sizeof(pixel_t); } if (n_colors 1 fg_count bg_count) { @@ -106,7 +106,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, n_data += 2; n_subtiles++; } - irow += ds_get_linesize(vs-ds) / sizeof(pixel_t); + irow += vnc_server_fb_stride(vd) / sizeof(pixel_t); } break; case 3: @@ -133,7 +133,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, has_color = 0; #ifdef GENERIC vnc_convert_pixel(vs, data + n_data, color); -n_data += vs-clientds.pf.bytes_per_pixel; +n_data += vs-client_pf.bytes_per_pixel; #else memcpy(data + n_data, color, sizeof(color)); n_data += sizeof(pixel_t); @@ -153,7 +153,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, if (has_color) { #ifdef GENERIC vnc_convert_pixel(vs, data + n_data, color); -n_data += vs-clientds.pf.bytes_per_pixel; +n_data += vs-client_pf.bytes_per_pixel; #else memcpy(data + n_data, color, sizeof(color)); n_data += sizeof(pixel_t); @@ -162,7 +162,7 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, n_data += 2; n_subtiles++; } - irow += ds_get_linesize(vs-ds) / sizeof(pixel_t); + irow += vnc_server_fb_stride(vd) / sizeof(pixel_t); } /* A SubrectsColoured subtile invalidates the foreground color */ @@ -190,18 +190,17 @@ static void CONCAT(send_hextile_tile_, NAME)(VncState *vs, vnc_write_u8(vs, flags); if (n_colors 4) { if (flags 0x02) - vs-write_pixels(vs, vd-server-pf, last_bg, sizeof(pixel_t)); + vs-write_pixels(vs, last_bg, sizeof(pixel_t)); if (flags 0x04) - vs-write_pixels(vs, vd-server-pf, last_fg, sizeof(pixel_t)); + vs-write_pixels(vs, last_fg, sizeof(pixel_t)); if (n_subtiles) { vnc_write_u8(vs, n_subtiles); vnc_write(vs, data, n_data); } } else { for (j = 0; j h; j++) { - vs-write_pixels(vs, vd-server-pf, row, - w * ds_get_bytes_per_pixel(vs-ds)); - row += ds_get_linesize(vs-ds); + vs-write_pixels(vs, row, w * 4); + row +=
Re: [Qemu-devel] [PATCH 08/25] migration (outgoing): add error propagation for all protocols
Paolo Bonzini pbonz...@redhat.com writes: Error propagation is already there for socket backends, even though it is (and remains) incomplete because no Error is passed to the NonBlockingConnectHandler. Why is that a problem? Add it to other protocols, simplifying code that tests for errors that will never happen. With all protocols understanding Error, the code can be simplified further by removing the return value. Before: (qemu) migrate fd: migrate: An undefined error has occurred (qemu) info migrate (qemu) After: (qemu) migrate fd: migrate: File descriptor named '' has not been found (qemu) info migrate capabilities: xbzrle: off Migration status: failed total time: 0 milliseconds Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- v1-v2: turn bizarre DPRINTF into an assertion failure or just drop it for the failure test of O_NONBLOCK. Clean up after this change. migration-fd.c | 19 --- migration-tcp.c | 13 ++--- migration-unix.c | 11 ++- migration.c | 17 ++--- migration.h | 9 - 6 file modificati, 22 inserzioni(+), 65 rimozioni(-) diff --git a/migration-exec.c b/migration-exec.c index 6c97db9..5f3f4b2 100644 --- a/migration-exec.c +++ b/migration-exec.c @@ -60,22 +60,18 @@ static int exec_close(MigrationState *s) return ret; } -int exec_start_outgoing_migration(MigrationState *s, const char *command) +void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp) { FILE *f; f = popen(command, w); if (f == NULL) { -DPRINTF(Unable to popen exec target\n); -goto err_after_popen; +error_setg_errno(errp, errno, failed to popen the migration target); +return; } s-fd = fileno(f); -if (s-fd == -1) { -DPRINTF(Unable to retrieve file descriptor for popen'd handle\n); -goto err_after_open; -} - +assert(s-fd != -1); Okay, because fileno() can fail only if its argument is not a valid stream, which really shouldn't be possible here. socket_set_nonblock(s-fd); s-opaque = qemu_popen(f, w); @@ -85,12 +81,6 @@ int exec_start_outgoing_migration(MigrationState *s, const char *command) s-write = file_write; migrate_fd_connect(s); -return 0; - -err_after_open: -pclose(f); -err_after_popen: -return -1; } static void exec_accept_incoming_migration(void *opaque) diff --git a/migration-fd.c b/migration-fd.c index 7335167..a7c800a 100644 --- a/migration-fd.c +++ b/migration-fd.c @@ -73,30 +73,19 @@ static int fd_close(MigrationState *s) return 0; } -int fd_start_outgoing_migration(MigrationState *s, const char *fdname) +void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **errp) { -s-fd = monitor_get_fd(cur_mon, fdname, NULL); +s-fd = monitor_get_fd(cur_mon, fdname, errp); if (s-fd == -1) { -DPRINTF(fd_migration: invalid file descriptor identifier\n); -goto err_after_get_fd; -} - -if (fcntl(s-fd, F_SETFL, O_NONBLOCK) == -1) { -DPRINTF(Unable to set nonblocking mode on file descriptor\n); -goto err_after_open; +return; } +fcntl(s-fd, F_SETFL, O_NONBLOCK); Sure this can't fail? s-get_error = fd_errno; s-write = fd_write; s-close = fd_close; migrate_fd_connect(s); -return 0; - -err_after_open: -close(s-fd); -err_after_get_fd: -return -1; } static void fd_accept_incoming_migration(void *opaque) [...]
Re: [Qemu-devel] [RFC PATCH 0/6] chardev: convert to QOM
Gerd Hoffmann kra...@redhat.com writes: Hi, I think the main decision point here is whether we introduce a separate chardev_add/chardev_del command or just use the qom-create command that has been posted previously. Do you have a git tree with this series + qom-create to look at and play with? https://github.com/aliguori/qemu/tree/chardev-qom.1 I haven't tried the qom-new command from Markus but I've also included the -object option that I posted a while ago. That seems to work. Here's an example command line: $ x86_64-softmmu/qemu-system-x86_64 \ -object chardev-file,path=foo.txt,label=chr0,id=chr0,realized=on \ -device isa-serial,index=0,chardev=chr0 -hda ~/images/linux.img \ -snapshot -enable-kvm A couple caveats: - The label/id duplication is ugly. The need to use label disappears though once we switch devices over to using links. - Some error checking is needed to deal with trying to use an unrealized chardev. - It may still make sense to have a chardev-add operation but I think that operation should work in terms of something like qom-new. Probably with some special casing to fix the container path to /backends/chardev and also to deal with automatically setting label based on id. It may make sense to also always set realized explicitly to 'on' in chardev-add. Regards, Anthony Liguori thanks, Gerd
Re: [Qemu-devel] [PATCHv4] qemu-img rebase: use empty string to rebase without backing file
Am 16.10.2012 14:46, schrieb Alex Bligh: This patch allows an empty filename to be passed as the new base image name for qemu-img rebase to mean base the image on no backing file (i.e. independent of any backing file). According to Eric Blake, qemu-img rebase already supports this when '-u' is used; this adds support when -u is not used. Signed-off-by: Alex Bligh a...@alex.org.uk --- qemu-img.c| 29 +++-- qemu-img.texi |4 +++- 2 files changed, 22 insertions(+), 11 deletions(-) Also obtainable from: https://github.com/abligh/qemu.git Commit at: https://github.com/abligh/qemu/commit/982ec105018f9901619663642de9d0a5de86799d diff --git a/qemu-img.c b/qemu-img.c index f17f187..2816f37 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1558,13 +1558,15 @@ static int img_rebase(int argc, char **argv) error_report(Could not open old backing file '%s', backing_name); goto out; } - -bs_new_backing = bdrv_new(new_backing); -ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS, +if (out_baseimg[0]) { +bs_new_backing = bdrv_new(new_backing); +ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS, new_backing_drv); -if (ret) { -error_report(Could not open new backing file '%s', out_baseimg); -goto out; +if (ret) { +error_report(Could not open new backing file '%s', + out_baseimg); +goto out; +} } } @@ -1580,7 +1582,7 @@ static int img_rebase(int argc, char **argv) if (!unsafe) { uint64_t num_sectors; uint64_t old_backing_num_sectors; -uint64_t new_backing_num_sectors; +uint64_t new_backing_num_sectors = 0; uint64_t sector; int n; uint8_t * buf_old; @@ -1592,7 +1594,9 @@ static int img_rebase(int argc, char **argv) bdrv_get_geometry(bs, num_sectors); bdrv_get_geometry(bs_old_backing, old_backing_num_sectors); -bdrv_get_geometry(bs_new_backing, new_backing_num_sectors); +if (bs_new_backing) { +bdrv_get_geometry(bs_new_backing, new_backing_num_sectors); +} local_progress = (float)100 / (num_sectors / MIN(num_sectors, IO_BUF_SIZE / 512)); @@ -1629,7 +1633,7 @@ static int img_rebase(int argc, char **argv) } } -if (sector = new_backing_num_sectors) { +if (sector = new_backing_num_sectors || !bs_new_backing) { memset(buf_new, 0, n * BDRV_SECTOR_SIZE); } else { if (sector + n new_backing_num_sectors) { @@ -1675,7 +1679,12 @@ static int img_rebase(int argc, char **argv) * backing file are overwritten in the COW file now, so the visible content * doesn't change when we switch the backing file. */ -ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt); +if (bs_new_backing) { I think this needs to be out_baseimg, otherwise -u is broken. I've updated the patch, please check if you agree with the fix. Kevin
Re: [Qemu-devel] [PATCH v3 0/2] qemu-img: Add --backing-chain option to info command
Am 17.10.2012 14:02, schrieb Stefan Hajnoczi: This series adds the --backing-chain option for enumerating the backing file chain. Given the topmost image it will print qemu-img info information for each image file in the chain. Special care needs to be taken when image files form an infinite loop. This is very unusual, most like due to malicious image files. Nevertheless, qemu-img must be robust against invalid inputs so we explicit check for this. v3: * Use ImageInfoList to avoid manually outputting JSON and to make the code more QAPI-friendly. * Solve the output vs error messages issues by printing a detailed error message but no image info. To dig into a broken image chain, rerun without --backing-chain. This is different from what I initially tried but is much cleaner because it doesn't produce confusing output. * rm -f $TEST_IMG.[123].base [Kevin] * Include --output=json in test case [Kevin] * Rename test case to free 043 number [Eric] Stefan Hajnoczi (2): qemu-img: Add --backing-chain option to info command qemu-iotests: Add 043 backing file chain infinite loop test qemu-img.c | 167 +++ tests/qemu-iotests/043 | 94 tests/qemu-iotests/043.out | 66 + tests/qemu-iotests/common.rc | 10 +++ tests/qemu-iotests/group | 1 + 5 files changed, 324 insertions(+), 14 deletions(-) create mode 100755 tests/qemu-iotests/043 create mode 100644 tests/qemu-iotests/043.out Thanks applied both to the block branch. Kevin
Re: [Qemu-devel] [PATCH] Memory-API: Make eventfd adhere to device endianness
On 10/15/2012 08:30 PM, Alexander Graf wrote: Our memory API MMIO regions know the concept of device endianness. This is used to automatically swap endianness between devices and host CPU, depending on whether buses in between would swizzle the bits. The ioeventfd value comparison does not adhere to that semantic though. Probably because nobody has been running ioeventfd on a BE platform and the only device implementing ioeventfd right now is LE (PCI) based. So add swizzling to ioeventfd registration / deletion to make the rest of the code as consistent as possible. Thanks a lot to Michael Tsirkin to point me towards the right direction. Thanks, applied. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH 08/25] migration (outgoing): add error propagation for all protocols
Il 17/10/2012 16:43, Markus Armbruster ha scritto: Error propagation is already there for socket backends, even though it is (and remains) incomplete because no Error is passed to the NonBlockingConnectHandler. Why is that a problem? It means that the exact error message still cannot be sent to the user if the OS reports it asynchronously via SO_ERROR. If NonBlockingConnectHandler received an Error**, we could for example report the error class and/or message via a new field of the query-migration command even if it is reported asynchronously. Paolo
Re: [Qemu-devel] [PATCH 11/25] nbd: ask and print error information from qemu-sockets
Paolo Bonzini pbonz...@redhat.com writes: Before: $ qemu-system-x86_64 nbd:localhost:12345 inet_connect_opts: connect(ipv4,yakj.usersys.redhat.com,127.0.0.1,12345): Connection refused qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused After: $ x86_64-softmmu/qemu-system-x86_64 nbd:localhost:12345 qemu-system-x86_64: Failed to connect to socket: Connection refused qemu-system-x86_64: could not open disk image nbd:localhost:12345: Connection refused Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- nbd.c | 39 +++ 1 file modificato, 31 inserzioni(+), 8 rimozioni(-) diff --git a/nbd.c b/nbd.c index f61a288..cec5a94 100644 --- a/nbd.c +++ b/nbd.c @@ -208,7 +208,14 @@ int tcp_socket_outgoing(const char *address, uint16_t port) int tcp_socket_outgoing_spec(const char *address_and_port) { -return inet_connect(address_and_port, NULL); +Error *local_err = NULL; +int fd = inet_connect(address_and_port, local_err); + +if (local_err != NULL) { +qerror_report_err(local_err); +error_free(local_err); +} +return fd; } int tcp_socket_incoming(const char *address, uint16_t port) @@ -220,22 +227,38 @@ int tcp_socket_incoming(const char *address, uint16_t port) int tcp_socket_incoming_spec(const char *address_and_port) { -char *ostr = NULL; -int olen = 0; -return inet_listen(address_and_port, ostr, olen, SOCK_STREAM, 0, NULL); +Error *local_err = NULL; +int fd = inet_listen(address_and_port, NULL, 0, SOCK_STREAM, 0, local_err); + +if (local_err != NULL) { +qerror_report_err(local_err); +error_free(local_err); +} +return fd; } int unix_socket_incoming(const char *path) { -char *ostr = NULL; -int olen = 0; +Error *local_err = NULL; +int fd = unix_listen(path, NULL, 0, local_err); -return unix_listen(path, ostr, olen, NULL); +if (local_err != NULL) { +qerror_report_err(local_err); +error_free(local_err); +} +return fd; } int unix_socket_outgoing(const char *path) { -return unix_connect(path, NULL); +Error *local_err = NULL; +int fd = unix_connect(path, local_err); + +if (local_err != NULL) { +qerror_report_err(local_err); +error_free(local_err); +} +return fd; } /* Basic flow for negotiation The boilerplate pattern Error *local_err = NULL; FOO(..., *local_err); if (local_err) { qerror_report_err(local_err); error_free(local_err); } is spreading. Not quite sure it's worth a macro.
Re: [Qemu-devel] [PATCH v3 0/2] qemu-img: Add --backing-chain option to info command
On 10/17/2012 08:47 AM, Kevin Wolf wrote: Am 17.10.2012 14:02, schrieb Stefan Hajnoczi: This series adds the --backing-chain option for enumerating the backing file chain. Given the topmost image it will print qemu-img info information for each image file in the chain. Thanks applied both to the block branch. Now we also need the documentation: https://lists.gnu.org/archive/html/qemu-devel/2012-10/msg02360.html -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/2] vnc: fix segfault due to failed handshake
Am 14.10.2012 15:08, schrieb Tim Hardeck: When the VNC server disconnects due to a failed handshake we don't have vs-bh allocated yet. Check for this case and don't delete it. Signed-off-by: Tim Hardeck thard...@suse.de --- ui/vnc.c |4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ui/vnc.c b/ui/vnc.c index 01b2daf..656895a 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -1055,7 +1055,9 @@ static void vnc_disconnect_finish(VncState *vs) vnc_unlock_output(vs); qemu_mutex_destroy(vs-output_mutex); -qemu_bh_delete(vs-bh); +if (vs-bh != NULL) { +qemu_bh_delete(vs-bh); +} buffer_free(vs-jobs_buffer); for (i = 0; i VNC_STAT_ROWS; ++i) { qemu_bh_delete() is not checking for a NULL argument, therefore this fix looks good to me, Acked-by: Andreas Färber afaer...@suse.de Adding some CCs. As a followup it might be a good idea to either assert or ignore a NULL argument in qemu_bh_delete(). Regards, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 11/25] nbd: ask and print error information from qemu-sockets
Il 17/10/2012 16:51, Markus Armbruster ha scritto: /* Basic flow for negotiation The boilerplate pattern Error *local_err = NULL; FOO(..., *local_err); if (local_err) { qerror_report_err(local_err); error_free(local_err); } is spreading. Not quite sure it's worth a macro. Actually this should not spread, but this one should: Error *local_err = NULL; FOO(..., *local_err); if (local_err) { error_propagate(errp, local_err); return; } Not quite sure how to macroize it though, at least without making the code too ugly to see. Paolo
Re: [Qemu-devel] [PATCH 2/2] qemu queue: fix uninitialized removals
Tim, Am 14.10.2012 15:08, schrieb Tim Hardeck: When calling QTAILQ_REMOVE or QLIST_REMOVE on an unitialized list QEMU segfaults. Can this be reproduced by a user today? Or is this just fixing the case that a developer forgot to initialize a list? Regards, Andreas Check for this case specifically on item removal. Signed-off-by: Tim Hardeck thard...@suse.de --- qemu-queue.h |8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/qemu-queue.h b/qemu-queue.h index 9288cd8..47ed239 100644 --- a/qemu-queue.h +++ b/qemu-queue.h @@ -141,7 +141,9 @@ struct { \ if ((elm)-field.le_next != NULL) \ (elm)-field.le_next-field.le_prev = \ (elm)-field.le_prev; \ -*(elm)-field.le_prev = (elm)-field.le_next; \ +if ((elm)-field.le_prev != NULL) { \ +*(elm)-field.le_prev = (elm)-field.le_next; \ +} \ } while (/*CONSTCOND*/0) #define QLIST_FOREACH(var, head, field) \ @@ -381,7 +383,9 @@ struct { \ (elm)-field.tqe_prev; \ else\ (head)-tqh_last = (elm)-field.tqe_prev; \ -*(elm)-field.tqe_prev = (elm)-field.tqe_next; \ +if ((elm)-field.tqe_prev != NULL) {\ +*(elm)-field.tqe_prev = (elm)-field.tqe_next; \ +} \ } while (/*CONSTCOND*/0) #define QTAILQ_FOREACH(var, head, field)\ -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCHv2 1/2] signal: added a wrapper for sigprocmask function
On 17 October 2012 15:18, Alex Barcelo abarc...@ac.upc.edu wrote: Create a wrapper for signal mask changes initiated by the guest; this will give us a place to put code which prevents the guest from changing the handling of signals used by QEMU itself internally. The wrapper is called from all the guest-initiated sigprocmask, but is not called from internal qemu sigprocmask calls. Signed-off-by: Alex Barcelo abarc...@ac.upc.edu In my comments on v1 of this patch I wrote: I think all the uses of sigprocmask() in linux-user/signal.c also need to be do_sigprocmask(), as they are the guest trying to control its signal mask (via the mask it specifies for running signal handlers, or the mask it passes back when restoring context on return from a signal handler). ...but I don't see those changes here. -- PMM
Re: [Qemu-devel] [PATCH v3 4/4] qemu-config: Add new -add-fd command line option
On 10/17/2012 08:02 AM, Kevin Wolf wrote: Am 17.10.2012 06:16, schrieb Eric Blake: I'm still seeing the corner case of: qemu-kvm -add-fd fd=3,set=1 -add-fd fd=4,set=2 4- where the dup(3) will populate fd 4 prior to the point where we get to process the -add-fd fd=4 command to notice that the user started qemu-kvm with fd 4 closed, and thus qemu will silently proceed to use the wrong fd. On the other hand, I'm not sure if that corner case is worth worrying about, or if we just chalk it up to user stupidity (aka libvirt programmer stupidity) if they did something like that (most likely, because the management app forgot to clear FD_CLOEXEC before exec()ing qemu-kvm). If you specify an FD number that isn't actually open when qemu is stared, you can get any FD that qemu opens internally. I think the correct answer to this problem is then don't do that. Overnight, I realized we do have one potential safety valve: we are guaranteed that any fd inherited by the exec() of qemu-kvm has FD_CLOEXEC clear, and we also strive to have qemu open/dup all of its internal fds with FD_CLOEXEC set. Therefore, it may be worth a sanity check of fcntl(F_GETFD) to see if FD_CLOEXEC is set, and if so, the user must have failed to pass in the fd, and we are now looking at a qemu internal fd, and should therefore report failure. But I'm not sure if it's worth the extra code. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature