[Qemu-devel] [PATCH 60/97] vmstate: VMSTATE_ARRAY_OF_POINTER didn't used the version_id field
Signed-off-by: Juan Quintela quint...@redhat.com --- include/migration/vmstate.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 5a26b6a..6d064de 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -296,9 +296,8 @@ extern const VMStateInfo vmstate_info_bitmap; .offset = vmstate_offset_pointer(_state, _field, _type), \ } -#define VMSTATE_ARRAY_OF_POINTER(_field, _state, _num, _version, _info, _type) {\ +#define VMSTATE_ARRAY_OF_POINTER(_field, _state, _num, _info, _type) {\ .name = (stringify(_field)), \ -.version_id = (_version),\ .num= (_num),\ .info = (_info), \ .size = sizeof(_type), \ @@ -553,7 +552,7 @@ extern const VMStateInfo vmstate_info_bitmap; VMSTATE_TIMER_TEST(_f, _s, NULL) #define VMSTATE_TIMER_ARRAY(_f, _s, _n) \ -VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer *) +VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, vmstate_info_timer, QEMUTimer*) #define VMSTATE_BOOL_ARRAY_TEST(_f, _s, _n, _t) \ VMSTATE_ARRAY(_f, _s, _n, _t, vmstate_info_bool, bool) -- 1.9.0
[Qemu-devel] [PATCH 47/97] vmstate: remove version from all variants of VMSTATE_STRUCT_POINTER*
There were no more users of them. Signed-off-by: Juan Quintela quint...@redhat.com --- include/migration/vmstate.h | 17 ++--- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 3f2084b..61d677e 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -310,18 +310,8 @@ extern const VMStateInfo vmstate_info_bitmap; .offset = vmstate_offset_value(_state, _field, _type), \ } -#define VMSTATE_STRUCT_POINTER_V(_field, _state, _version, _vmsd, _type) { \ +#define VMSTATE_STRUCT_POINTER_TEST(_field, _state, _test, _vmsd, _type) { \ .name = (stringify(_field)), \ -.version_id = (_version),\ -.vmsd = (_vmsd),\ -.size = sizeof(_type *), \ -.flags= VMS_STRUCT|VMS_POINTER, \ -.offset = vmstate_offset_pointer(_state, _field, _type), \ -} - -#define VMSTATE_STRUCT_POINTER_TEST_V(_field, _state, _test, _version, _vmsd, _type) { \ -.name = (stringify(_field)), \ -.version_id = (_version),\ .field_exists = (_test), \ .vmsd = (_vmsd),\ .size = sizeof(_type *), \ @@ -516,10 +506,7 @@ extern const VMStateInfo vmstate_info_bitmap; VMSTATE_STRUCT_TEST(_field, _state, NULL, _version, _vmsd, _type) #define VMSTATE_STRUCT_POINTER(_field, _state, _vmsd, _type) \ -VMSTATE_STRUCT_POINTER_V(_field, _state, 0, _vmsd, _type) - -#define VMSTATE_STRUCT_POINTER_TEST(_field, _state, _test, _vmsd, _type) \ -VMSTATE_STRUCT_POINTER_TEST_V(_field, _state, _test, 0, _vmsd, _type) +VMSTATE_STRUCT_POINTER_TEST(_field, _state, NULL, _vmsd, _type) #define VMSTATE_STRUCT_ARRAY(_field, _state, _num, _version, _vmsd, _type) \ VMSTATE_STRUCT_ARRAY_TEST(_field, _state, _num, NULL, _version, \ -- 1.9.0
[Qemu-devel] [PATCH 43/97] vmstate: Remove unused VMSTATE_BUFFER_V
Signed-off-by: Juan Quintela quint...@redhat.com --- include/migration/vmstate.h | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index e7ceb59..fbf7dbe 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -654,11 +654,8 @@ extern const VMStateInfo vmstate_info_bitmap; #define VMSTATE_UINT32_SUB_ARRAY(_f, _s, _start, _num)\ VMSTATE_SUB_ARRAY(_f, _s, _start, _num, 0, vmstate_info_uint32, uint32_t) -#define VMSTATE_BUFFER_V(_f, _s, _v) \ -VMSTATE_STATIC_BUFFER(_f, _s, _v, NULL, 0, sizeof(typeof_field(_s, _f))) - #define VMSTATE_BUFFER(_f, _s)\ -VMSTATE_BUFFER_V(_f, _s, 0) +VMSTATE_STATIC_BUFFER(_f, _s, 0, NULL, 0, sizeof(typeof_field(_s, _f))) #define VMSTATE_PARTIAL_BUFFER(_f, _s, _size) \ VMSTATE_STATIC_BUFFER(_f, _s, 0, NULL, 0, _size) -- 1.9.0
[Qemu-devel] [PATCH 65/97] vmstate: Remove version parameter from VMSTATE_STATIC_BUFFER
No user for it. Signed-off-by: Juan Quintela quint...@redhat.com --- include/migration/vmstate.h | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index b9d5a28..95e4e17 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -381,9 +381,8 @@ extern const VMStateInfo vmstate_info_bitmap; .offset = offsetof(_state, _field), \ } -#define VMSTATE_STATIC_BUFFER(_field, _state, _version, _test, _start, _size) { \ +#define VMSTATE_STATIC_BUFFER(_field, _state, _test, _start, _size) { \ .name = (stringify(_field)), \ -.version_id = (_version), \ .field_exists = (_test), \ .size = (_size - _start),\ .info = vmstate_info_buffer,\ @@ -591,13 +590,13 @@ extern const VMStateInfo vmstate_info_bitmap; VMSTATE_SUB_ARRAY(_f, _s, _start, _num, vmstate_info_uint32, uint32_t) #define VMSTATE_BUFFER(_f, _s)\ -VMSTATE_STATIC_BUFFER(_f, _s, 0, NULL, 0, sizeof(typeof_field(_s, _f))) +VMSTATE_STATIC_BUFFER(_f, _s, NULL, 0, sizeof(typeof_field(_s, _f))) #define VMSTATE_PARTIAL_BUFFER(_f, _s, _size) \ -VMSTATE_STATIC_BUFFER(_f, _s, 0, NULL, 0, _size) +VMSTATE_STATIC_BUFFER(_f, _s, NULL, 0, _size) #define VMSTATE_BUFFER_START_MIDDLE(_f, _s, _start) \ -VMSTATE_STATIC_BUFFER(_f, _s, 0, NULL, _start, sizeof(typeof_field(_s, _f))) +VMSTATE_STATIC_BUFFER(_f, _s, NULL, _start, sizeof(typeof_field(_s, _f))) #define VMSTATE_PARTIAL_VBUFFER(_f, _s, _size)\ VMSTATE_VBUFFER(_f, _s, NULL, 0, _size) @@ -609,7 +608,7 @@ extern const VMStateInfo vmstate_info_bitmap; VMSTATE_VBUFFER(_f, _s, NULL, _start, _size) #define VMSTATE_BUFFER_TEST(_f, _s, _test)\ -VMSTATE_STATIC_BUFFER(_f, _s, 0, _test, 0, sizeof(typeof_field(_s, _f))) +VMSTATE_STATIC_BUFFER(_f, _s, _test, 0, sizeof(typeof_field(_s, _f))) #define VMSTATE_BUFFER_UNSAFE(_field, _state, _size) \ VMSTATE_BUFFER_UNSAFE_TEST(_field, _state, NULL, vmstate_info_buffer, _size) -- 1.9.0
[Qemu-devel] [PATCH 08/97] vmstate: Remove VMSTATE_UINTL_EQUAL_V
It was not used anywhere. Signed-off-by: Juan Quintela quint...@redhat.com --- include/hw/hw.h | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/include/hw/hw.h b/include/hw/hw.h index 33bdb92..2b68ac3 100644 --- a/include/hw/hw.h +++ b/include/hw/hw.h @@ -51,22 +51,20 @@ int qemu_boot_set(const char *boot_order); #if TARGET_LONG_BITS == 64 #define VMSTATE_UINTTL_V(_f, _s, _v) \ VMSTATE_UINT64_V(_f, _s, _v) -#define VMSTATE_UINTTL_EQUAL_V(_f, _s, _v)\ -VMSTATE_UINT64_EQUAL_V(_f, _s, _v) +#define VMSTATE_UINTTL_EQUAL(_f, _s) \ +VMSTATE_UINT64_EQUAL(_f, _s) #define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)\ VMSTATE_UINT64_ARRAY_V(_f, _s, _n, _v) #else #define VMSTATE_UINTTL_V(_f, _s, _v) \ VMSTATE_UINT32_V(_f, _s, _v) -#define VMSTATE_UINTTL_EQUAL_V(_f, _s, _v)\ -VMSTATE_UINT32_EQUAL_V(_f, _s, _v) +#define VMSTATE_UINTTL_EQUAL(_f, _s) \ +VMSTATE_UINT32_EQUAL(_f, _s) #define VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, _v)\ VMSTATE_UINT32_ARRAY_V(_f, _s, _n, _v) #endif #define VMSTATE_UINTTL(_f, _s)\ VMSTATE_UINTTL_V(_f, _s, 0) -#define VMSTATE_UINTTL_EQUAL(_f, _s) \ -VMSTATE_UINTTL_EQUAL_V(_f, _s, 0) #define VMSTATE_UINTTL_ARRAY(_f, _s, _n) \ VMSTATE_UINTTL_ARRAY_V(_f, _s, _n, 0) -- 1.9.0
[Qemu-devel] [Bug 1062220] Re: qemu-system-arm crashed with SIGABRT in cpu_abort()
** Tags added: trusty -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1062220 Title: qemu-system-arm crashed with SIGABRT in cpu_abort() Status in QEMU: New Status in “qemu” package in Ubuntu: Incomplete Status in “qemu-linaro” package in Ubuntu: Incomplete Bug description: -kernel u-boot.bin ProblemType: Crash DistroRelease: Ubuntu 12.10 Package: qemu-system 1.2.0-2012.09-0ubuntu1 ProcVersionSignature: Ubuntu 3.5.0-10.10-generic 3.5.1 Uname: Linux 3.5.0-10-generic x86_64 NonfreeKernelModules: nvidia ApportVersion: 2.6.1-0ubuntu1 Architecture: amd64 CrashCounter: 1 Date: Fri Oct 5 19:30:23 2012 ExecutablePath: /usr/bin/qemu-system-arm InstallationMedia: Ubuntu 11.10 Oneiric Ocelot - Alpha amd64 (20110804) ProcCmdline: qemu-system-arm -M versatilepb -kernel u-boot.bin Signal: 6 SourcePackage: qemu-linaro StacktraceTop: raise () from /lib/x86_64-linux-gnu/libc.so.6 abort () from /lib/x86_64-linux-gnu/libc.so.6 ?? () ?? () ?? () Title: qemu-system-arm crashed with SIGABRT in raise() UpgradeStatus: Upgraded to quantal on 2012-08-11 (54 days ago) UserGroups: adm admin cdrom dialout lpadmin plugdev sambashare vboxusers To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1062220/+subscriptions
Re: [Qemu-devel] E820 (Re: [v4 PATCH 00/12] SMBIOS: build full tables in QEMU)
On Do, 2014-04-03 at 11:42 +0200, Laszlo Ersek wrote: (a) Gerd's packages: http://www.kraxel.org/repos/ Under (a) you find some short instructions, and a set of RPMs that is automatically rebuilt twice a day (IIRC). It polls the git repos once per hour and kicks a build on new commits. So usually it should not take more than two hours from commit to updated packages. cheers, Gerd
Re: [Qemu-devel] [PATCH 08/35] qdev: hotplug for buss-less devices
On Mon, 07 Apr 2014 12:26:23 +1000 Alexey Kardashevskiy a...@ozlabs.ru wrote: On 04/05/2014 12:36 AM, Igor Mammedov wrote: Adds get_hotplug_handler() method to machine, and makes bus-less device to use it during hotplug as a means to discover hotplug handler controller. Returned controller is used to permorm a hotplug action. returned controller is a machine here? not necessary, it could be any device that implements HotplugHandler interface. Machine only provides simple means to get it is a simplified version of https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg04356.html as suggested by Paolo. Why not to make a memory controller + bus instead? I thought this is the preferred approach in qom'ed QEMU :) It's generic mechanism that could be used for hotplugging any bus-less device. As regarding bus approach that was used for memory hotplug in V7, Andreas've made suggestion to get rid of it as it doesn't map well for generic DIMM device and might be problematic to reuse on other targets. In V7 bus was used only as a means to discover a hotplug handler, with this path bus is not needed anymore. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/core/qdev.c | 13 + include/hw/boards.h | 8 2 files changed, 21 insertions(+) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 60f9df1..50bb8f5 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -34,6 +34,7 @@ #include qapi/qmp/qjson.h #include monitor/monitor.h #include hw/hotplug.h +#include hw/boards.h int qdev_hotplug = 0; static bool qdev_hot_added = false; @@ -761,6 +762,18 @@ static void device_set_realized(Object *obj, bool value, Error **err) local_err == NULL) { hotplug_handler_plug(dev-parent_bus-hotplug_handler, dev, local_err); +} else if (local_err == NULL + object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) { +HotplugHandler *hotplug_ctrl; +MachineState *machine = MACHINE(qdev_get_machine()); +MachineClass *mc = MACHINE_GET_CLASS(machine); + +if (mc-get_hotplug_handler) { +hotplug_ctrl = mc-get_hotplug_handler(machine, dev); +if (hotplug_ctrl) { +hotplug_handler_plug(hotplug_ctrl, dev, local_err); +} +} } if (qdev_get_vmsd(dev) local_err == NULL) { diff --git a/include/hw/boards.h b/include/hw/boards.h index 3567190..67750b5 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -73,6 +73,11 @@ extern MachineState *current_machine; /** * MachineClass: * @qemu_machine: #QEMUMachine + * @get_hotplug_handler: this function is called during bus-less + *device hotplug. If defined it returns pointer to an instance + *of HotplugHandler object, which handles hotplug operation + *for a given @dev. It may return NULL if @dev doesn't require + *any actions to be performed by hotplug handler. */ struct MachineClass { /* private */ @@ -80,6 +85,9 @@ struct MachineClass { /* public */ QEMUMachine *qemu_machine; + +HotplugHandler *(*get_hotplug_handler)(MachineState *machine, + DeviceState *dev); }; /**
Re: [Qemu-devel] [PATCH 14/35] pc: exit QEMU if slots 256
On Fri, 4 Apr 2014 14:14:04 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Fri, Apr 04, 2014 at 03:36:39PM +0200, Igor Mammedov wrote: ... which is current ACPI implementation limit. Signed-off-by: Igor Mammedov imamm...@redhat.com With this we will have yet another hardcoded limit/assumption scattererd in different places around the code, which we won't know how/if it can be changed in the future. I would prefer to have a #define and an explanation on the code for the specific limit, so people know if/when/how it is safe to change it a few years from now. Something like: hw/acpi/acpi.h: /* current device naming scheme dosen't support * more that 256 memory devices */ #define ACPI_MAX_RAM_SLOTS 256 hw/acpi/acpi-build.c:build_ssdt(): assert(nr_mem = ACPI_MAX_RAM_SLOTS) hw/i386/pc.c: if (machine-init_args.ram_slots ACPI_MAX_RAM_SLOTS) { error_report(unsupported amount of memory slots: %PRIu64, machine-init_args.ram_slots); exit(EXIT_FAILURE); } sure, I'll fix it for the next series respin. --- hw/i386/pc.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 69e4225..6fe1803 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1204,6 +1204,12 @@ FWCfgState *pc_memory_init(MemoryRegion *system_memory, ram_addr_t hotplug_mem_size = machine-init_args.maxram_size - ram_size; +if (machine-init_args.ram_slots 256) { +error_report(unsupported amount of memory slots: %PRIu64, + machine-init_args.ram_slots); +exit(EXIT_FAILURE); +} + pcms-hotplug_memory_base = ROUND_UP(0x1ULL + above_4g_mem_size, 1ULL 30); -- 1.9.0
Re: [Qemu-devel] E820 (Re: [v4 PATCH 00/12] SMBIOS: build full tables in QEMU)
On Do, 2014-04-03 at 09:32 -0400, Gabriel L. Somlo wrote: For that, ideally, I'd like to clone a git repo (and pull once every few days to stay on top of things). Looks like the top-level upstream one doesn't have your smbios code, so would there be a mid-stream (for the lack of a better term) git repo you have that I can clone and hack against for the smbios stuff ? http://www.kraxel.org/cgit/jenkins/edk2/tree/ spec file and patches. specfile is patched by the jenkins build scripts, so a manual rpm build will need some fiddeling, but I guess you are more interested in the patches ;) cheers, Gerd
Re: [Qemu-devel] [PATCH 30/35] pc: ACPI BIOS: name CPU hotplug ACPI0004 device
On Sun, 6 Apr 2014 12:18:50 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 04, 2014 at 03:36:55PM +0200, Igor Mammedov wrote: Following patches will add another ACPI0004 device to the same scope, and that will make Windows BSOD because it thinks that the second ACPI0004 device is duplicate. Adding to device unique _UID, fixes issue and allows Windows to distinguish devices with the same _HID Signed-off-by: Igor Mammedov imamm...@redhat.com Weren't we going to rename ACPI0004 to PNP0C02 or something? I remember it creates problems for older guests. I've tested it wit XPsp3 and WS2003, and they are fine with it. The reason why ACPI0004 didn't worked nice at the beginning was that IO ranges that were exposed as _CRS in it were conflicting with PCI0._CRS. With Q35 CPU hotplug patches that was fixed and ACPI0004 works just fine. PNP0C02 has a small drawback, which is that OSPM (Windows) doesn't do _CRS verification (i.e. ignores it), so lets keep it as the last resort/workaround to use if nothing else works. --- hw/i386/acpi-dsdt-cpu-hotplug.dsl | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl index dee4843..81fb876 100644 --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl @@ -94,6 +94,7 @@ Scope(\_SB) { Device(CPU_HOTPLUG_RESOURCE_DEVICE) { Name(_HID, ACPI0004) +Name(_UID, CPU hotplug resources) Name(_CRS, ResourceTemplate() { IO(Decode16, CPU_STATUS_BASE, CPU_STATUS_BASE, 0, CPU_STATUS_LEN) -- 1.9.0
Re: [Qemu-devel] [PATCH v5 23/24] usb: sanity check setup_index+setup_len in post_load
On Do, 2014-04-03 at 19:52 +0300, Michael S. Tsirkin wrote: -if (dev-setup_index = sizeof(dev-data_buf) || +if (dev-setup_index 0 || +dev-setup_len 0 || +dev-setup_index = sizeof(dev-data_buf) || dev-setup_len = sizeof(dev-data_buf)) { return -EINVAL; } Reviewed-by: Gerd Hoffmann kra...@redhat.com
Re: [Qemu-devel] [PATCH 31/35] pc: ACPI BIOS: implement memory hotplug interface
On Sun, 6 Apr 2014 12:13:36 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 04, 2014 at 03:36:56PM +0200, Igor Mammedov wrote: - provides static SSDT object for memory hotplug - SSDT template for memory devices and runtime generator of them in SSDT table. Signed-off-by: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/Makefile.objs | 3 +- hw/i386/acpi-build.c | 37 hw/i386/ssdt-mem.dsl | 75 +++ hw/i386/ssdt-misc.dsl | 163 ++ 4 files changed, 277 insertions(+), 1 deletion(-) create mode 100644 hw/i386/ssdt-mem.dsl diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs index 3df1612..fd04fc2 100644 --- a/hw/i386/Makefile.objs +++ b/hw/i386/Makefile.objs @@ -9,7 +9,8 @@ obj-y += acpi-build.o obj-y += bios-linker-loader.o hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \ hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \ - hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex + hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \ + hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex iasl-option=$(shell if test -z `$(1) $(2) 21 /dev/null` \ ; then echo $(2); else echo $(3); fi ;) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index a5d3fbf..6649480 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -37,6 +37,7 @@ #include bios-linker-loader.h #include hw/loader.h #include hw/isa/isa.h +#include hw/acpi/memory_hotplug.h /* Supported chipsets: */ #include hw/acpi/piix4.h @@ -664,6 +665,14 @@ static inline char acpi_get_hex(uint32_t val) #define ACPI_PCIQXL_SIZEOF (*ssdt_pciqxl_end - *ssdt_pciqxl_start) #define ACPI_PCIQXL_AML (ssdp_pcihp_aml + *ssdt_pciqxl_start) +#include hw/i386/ssdt-mem.hex + +/* 0x5B 0x82 DeviceOp PkgLength NameString DimmID */ +#define ACPI_MEM_OFFSET_HEX (*ssdt_mem_name - *ssdt_mem_start + 2) +#define ACPI_MEM_OFFSET_ID (*ssdt_mem_id - *ssdt_mem_start + 7) +#define ACPI_MEM_SIZEOF (*ssdt_mem_end - *ssdt_mem_start) +#define ACPI_MEM_AML (ssdm_mem_aml + *ssdt_mem_start) + #define ACPI_SSDT_SIGNATURE 0x54445353 /* SSDT */ #define ACPI_SSDT_HEADER_LENGTH 36 @@ -999,6 +1008,8 @@ build_ssdt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc, PcPciInfo *pci, PcGuestInfo *guest_info) { +MachineState *machine = MACHINE(qdev_get_machine()); +uint32_t nr_mem = machine-init_args.ram_slots; unsigned acpi_cpus = guest_info-apic_id_limit; int ssdt_start = table_data-len; uint8_t *ssdt_ptr; @@ -1027,6 +1038,9 @@ build_ssdt(GArray *table_data, GArray *linker, ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml), ssdt_isa_pest[0], 16, misc-pvpanic_port); +ACPI_BUILD_SET_LE(ssdt_ptr, sizeof(ssdp_misc_aml), + ssdt_mctrl_nr_slots[0], 32, nr_mem); + { GArray *sb_scope = build_alloc_array(); uint8_t op = 0x10; /* ScopeOp */ @@ -1080,6 +1094,29 @@ build_ssdt(GArray *table_data, GArray *linker, build_free_array(package); } +if (nr_mem) { +/* + * current device naming scheme dosen't support doesn't + * more that 256 memory devices + */ +assert(nr_mem = 256); +/* build memory devices */ +for (i = 0; i nr_mem; i++) { +char id[3]; +uint8_t *mem = acpi_data_push(sb_scope, ACPI_MEM_SIZEOF); + +snprintf(id, sizeof(id), %02X, i); +memcpy(mem, ACPI_MEM_AML, ACPI_MEM_SIZEOF); +memcpy(mem + ACPI_MEM_OFFSET_HEX, id, 2); +memcpy(mem + ACPI_MEM_OFFSET_ID, id, 2); +} + +/* build Method(MTFY, 2) { + * If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ... + */ +build_append_notify_method(sb_scope, MTFY, MP%0.02X, nr_mem); +} + { AcpiBuildPciBusHotplugState hotplug_state; Object *pci_host; diff --git a/hw/i386/ssdt-mem.dsl b/hw/i386/ssdt-mem.dsl new file mode 100644 index 000..7f68750 --- /dev/null +++ b/hw/i386/ssdt-mem.dsl @@ -0,0 +1,75 @@ +/* + * Memory hotplug ACPI DSDT static objects definitions + * + * Copyright ProfitBricks GmbH 2012 + * Copyright (C) 2013 Red Hat Inc + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This
Re: [Qemu-devel] [PATCH 00/35] pc: ACPI memory hotplug
On Fri, 4 Apr 2014 17:57:28 +0100 Dr. David Alan Gilbert dgilb...@redhat.com wrote: * Igor Mammedov (imamm...@redhat.com) wrote: snip This series allows to hotplug 'arbitrary' DIMM devices specifying size, NUMA node mapping (guest side), slot and address where to map it, at runtime. Some high level questions: 1) Is the intention that all guest RAM would be hot pluggable like this (i.e. no memory would be allocated in the normal way) Later, I plan to convert initial memory to DIMM devices as well, but only to not hotpluggable ones so far for simplicity sake. 2) Does something stop it being invoked during a migration? As far as I know, there is no checks to prevent any hotplug op during migration. Considering how migration currently works, hotplug should be disabled at migration time. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH for-2.0] dsdt: tweak ACPI ID for hotplug resource device
On Sun, 6 Apr 2014 12:47:37 +0300 Michael S. Tsirkin m...@redhat.com wrote: ACPI0004 seems too new: Windows XP complains about an unrecognized device. This is a regression since 1.7. Use PNP0A06 instead - Generic Container Device. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Note: Igor has RFC patches to use PNP0C02 under PCI0, but that's not ready for 2.0. Igor, could you comment on whether PNP0C02 is preferable for some OS-es? For PNP0A06 ACPI spec says: PNP0A06 Generic Container Device. This device behaves exactly the same as the PNP0A05 device. This was originally known as Extended I/O Bus. This ID should only be used for containers that do not produce resources for consumption by child devices. Any system resources claimed by a PNP0A06 device’s _CRS object must be consumed by the container itself. PNP0C02 is not in ACPI spec. It does appear in MicroSoft legacy PNP ID document: PNP0C02 General ID for reserving resources required by Plug and Play motherboard registers. (Not specific to a particular device.) Thoughts? From testing Windows doesn't check for conflicts _CRS provided by PNP0C02, so while using it will let XP not to complain about unknown device, it will also make later Windows OSes silently ignore conflicts if any. When XP's complaining for the first time about unknown device, it could be told to ignore it, so it would stop to complain in future. i.e. user have to silence only once. Wouldn't it be better to use ACPI0004 that gives minor annoyance to XP user but provides error checking with later OS versions? hw/i386/acpi-dsdt-cpu-hotplug.dsl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl index dee4843..34aab5a 100644 --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl @@ -93,7 +93,7 @@ Scope(\_SB) { } Device(CPU_HOTPLUG_RESOURCE_DEVICE) { -Name(_HID, ACPI0004) +Name(_HID, EisaId(PNP0A06)) Name(_CRS, ResourceTemplate() { IO(Decode16, CPU_STATUS_BASE, CPU_STATUS_BASE, 0, CPU_STATUS_LEN)
Re: [Qemu-devel] E820 (Re: [v4 PATCH 00/12] SMBIOS: build full tables in QEMU)
Hi, The only fly in this ointment may be that type 0 doesn't have a fixed length that could be edited in place, if you consider the various strings that get tacked on to the end of it. So you'd still have to slide the rest of the smbios payload left or right to shrink or enlarge the type 0 blob, depending on how you modify the various strings it contains... The dummy type 0 subtable that QEMU generates can have dummy space padded strings that the firmware can overwrite. Until recently, the max size smbios string was 64 bytes, so that size could be used. (As above, I admit that this is ugly, but the alternatives also seem ugly.) Another option would be to just leave the strings at a QEMU default as that's no different from what SeaBIOS does today. I don't think we need to make it that complicated. smbios tables don't have any references, right? I mean any references which would need a fixup (such as table pointers in RSDP in acpi) and therefore would need the romfile_loader. The string references within a table are relative don't need special care. Gabriel has code to generate all tables needed in qemu meanwhile, so I think we can simply have a blob in fw_cfg with all tables (except type0). firmware generates type0 table like it does today, then simply appends the fw_cfg blob as-is, then appends a end-of-tables marker. Done. OVMF probably would have to parse the blob, split it into tables, then install them one by one. But I suspect that will be less code than dealing with the complex smbios fw_cfg interface we have today ... cheers, Gerd
Re: [Qemu-devel] [PATCH v3 01/26] tcg-aarch64: Properly detect SIGSEGV writes
On 03.04.2014 21:56, Richard Henderson wrote: Since the kernel doesn't pass any info on the reason for the fault, disassemble the instruction to detect a store. Signed-off-by: Richard Henderson r...@twiddle.net --- user-exec.c | 29 +++-- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/user-exec.c b/user-exec.c index bc58056..52f76c9 100644 --- a/user-exec.c +++ b/user-exec.c @@ -465,16 +465,33 @@ int cpu_signal_handler(int host_signum, void *pinfo, #elif defined(__aarch64__) -int cpu_signal_handler(int host_signum, void *pinfo, - void *puc) +int cpu_signal_handler(int host_signum, void *pinfo, void *puc) { siginfo_t *info = pinfo; struct ucontext *uc = puc; -uint64_t pc; -int is_write = 0; /* XXX how to determine? */ +uintptr_t pc = uc-uc_mcontext.pc; +uint32_t insn = *(uint32_t *)pc; +bool is_write; -pc = uc-uc_mcontext.pc; -return handle_cpu_signal(pc, (uint64_t)info-si_addr, +/* XXX: need kernel patch to get write flag faster. */ +/* XXX: several of these could be combined. */ +is_write = ( (insn 0xbfff) == 0x0c00 /* C3.3.1 */ +|| (insn 0xbfe0) == 0x0c80 /* C3.3.2 */ +|| (insn 0xbfdf) == 0x0d00 /* C3.3.3 */ +|| (insn 0xbfc0) == 0x0d80 /* C3.3.4 */ +|| (insn 0x3f40) == 0x0800 /* C3.3.6 */ +|| (insn 0x3bc0) == 0x2840 /* C3.3.7 */ I think the Load (L) bit should be 0 here so == 0x2800 +|| (insn 0x3be00c00) == 0x38000400 /* C3.3.8 */ With V=1, an opc of 0b10 is also a write, I think. It's the 128bit FP/SIMD STR. +|| (insn 0x3be00c00) == 0x38000c00 /* C3.3.9 */ Same here. +|| (insn 0x3be00c00) == 0x38200800 /* C3.3.10 */ Same. +|| (insn 0x3be00c00) == 0x38000800 /* C3.3.11 */ +|| (insn 0x3be00c00) == 0x3800 /* C3.3.12 */ Same. +|| (insn 0x3bc0) == 0x3900 /* C3.3.13 */ Same. +|| (insn 0x3bc0) == 0x2900 /* C3.3.14 */ +|| (insn 0x3bc0) == 0x2880 /* C3.3.15 */ +|| (insn 0x3bc0) == 0x2980); /* C3.3.16 */ + +return handle_cpu_signal(pc, (uintptr_t)info-si_addr, is_write, uc-uc_sigmask, puc); } Thanks, Claudio
Re: [Qemu-devel] [PATCH v2 1/2] usb-hid: Add high speed mouse configuration
On Do, 2014-04-03 at 01:27 -0400, Ján Veselý wrote: ping2 is there anything I can do to help these patches get merged? Well, v2 is a step into the right direction but not complete yet. The usb_ver property hasn't any effect for mouse and keyboard. Also we'll need a compat property so older machine types continue to have usb1 mouse+keyboard. The compat property stuff is a bit tricky so my plan was to just do that myself, but I havn't found the time yet. If you wanna tackle the challenge have a look at 427e3aa151c749225364d0c30640e2e3c1756d9d (tablet patch). keyboard+mouse need to do the same. Also some cleanups can be done, such as moving the switch (us-usb_version) { ... } from usb_tablet_initfn to usb_hid_initfn to avoid duplicating it. cheers, Gerd
Re: [Qemu-devel] [PATCH v3 11/26] tcg-aarch64: Reuse LR in translated code
On 03.04.2014 21:56, Richard Henderson wrote: It's obviously call-clobbered, but is otherwise unused. Repurpose it as the TCG temporary. Signed-off-by: Richard Henderson r...@twiddle.net --- tcg/aarch64/tcg-target.c | 34 -- tcg/aarch64/tcg-target.h | 32 +--- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c index 48a246d..e36909e 100644 --- a/tcg/aarch64/tcg-target.c +++ b/tcg/aarch64/tcg-target.c @@ -23,10 +23,7 @@ static const char * const tcg_target_reg_names[TCG_TARGET_NB_REGS] = { %x0, %x1, %x2, %x3, %x4, %x5, %x6, %x7, %x8, %x9, %x10, %x11, %x12, %x13, %x14, %x15, %x16, %x17, %x18, %x19, %x20, %x21, %x22, %x23, -%x24, %x25, %x26, %x27, %x28, -%fp, /* frame pointer */ -%lr, /* link register */ -%sp, /* stack pointer */ +%x24, %x25, %x26, %x27, %x28, %fp, %x30, %sp, }; #endif /* NDEBUG */ @@ -41,16 +38,17 @@ static const int tcg_target_reg_alloc_order[] = { TCG_REG_X24, TCG_REG_X25, TCG_REG_X26, TCG_REG_X27, TCG_REG_X28, /* we will reserve this for GUEST_BASE if configured */ -TCG_REG_X9, TCG_REG_X10, TCG_REG_X11, TCG_REG_X12, -TCG_REG_X13, TCG_REG_X14, TCG_REG_X15, +TCG_REG_X8, TCG_REG_X9, TCG_REG_X10, TCG_REG_X11, +TCG_REG_X12, TCG_REG_X13, TCG_REG_X14, TCG_REG_X15, TCG_REG_X16, TCG_REG_X17, -TCG_REG_X18, TCG_REG_X19, /* will not use these, see tcg_target_init */ - TCG_REG_X0, TCG_REG_X1, TCG_REG_X2, TCG_REG_X3, TCG_REG_X4, TCG_REG_X5, TCG_REG_X6, TCG_REG_X7, -TCG_REG_X8, /* will not use, see tcg_target_init */ +/* X18 reserved by system */ +/* X19 reserved for AREG0 */ +/* X29 reserved as fp */ +/* X30 reserved as temporary */ }; static const int tcg_target_call_iarg_regs[8] = { @@ -61,13 +59,13 @@ static const int tcg_target_call_oarg_regs[1] = { TCG_REG_X0 }; -#define TCG_REG_TMP TCG_REG_X8 +#define TCG_REG_TMP TCG_REG_X30 #ifndef CONFIG_SOFTMMU -# if defined(CONFIG_USE_GUEST_BASE) -# define TCG_REG_GUEST_BASE TCG_REG_X28 +# ifdef CONFIG_USE_GUEST_BASE +# define TCG_REG_GUEST_BASE TCG_REG_X28 # else -# define TCG_REG_GUEST_BASE TCG_REG_XZR +# define TCG_REG_GUEST_BASE TCG_REG_XZR # endif #endif @@ -1871,7 +1869,7 @@ static void tcg_target_init(TCGContext *s) (1 TCG_REG_X12) | (1 TCG_REG_X13) | (1 TCG_REG_X14) | (1 TCG_REG_X15) | (1 TCG_REG_X16) | (1 TCG_REG_X17) | - (1 TCG_REG_X18)); + (1 TCG_REG_X18) | (1 TCG_REG_X30)); tcg_regset_clear(s-reserved_regs); tcg_regset_set_reg(s-reserved_regs, TCG_REG_SP); @@ -1902,13 +1900,13 @@ static void tcg_target_qemu_prologue(TCGContext *s) tcg_out_push_pair(s, TCG_REG_SP, TCG_REG_FP, TCG_REG_LR, frame_size_callee_saved); -/* FP - callee_saved */ +/* Set up frame pointer for canonical unwinding. */ tcg_out_movr_sp(s, TCG_TYPE_I64, TCG_REG_FP, TCG_REG_SP); -/* store callee-preserved regs x19..x28 using FP - callee_saved */ +/* Store callee-preserved regs x19..x28. */ for (r = TCG_REG_X19; r = TCG_REG_X27; r += 2) { int idx = (r - TCG_REG_X19) / 2 + 1; -tcg_out_store_pair(s, TCG_REG_FP, r, r + 1, idx); +tcg_out_store_pair(s, TCG_REG_SP, r, r + 1, idx); } /* Make stack space for TCG locals. */ @@ -1939,7 +1937,7 @@ static void tcg_target_qemu_prologue(TCGContext *s) FP must be preserved, so it still points to callee_saved area */ for (r = TCG_REG_X19; r = TCG_REG_X27; r += 2) { int idx = (r - TCG_REG_X19) / 2 + 1; -tcg_out_load_pair(s, TCG_REG_FP, r, r + 1, idx); +tcg_out_load_pair(s, TCG_REG_SP, r, r + 1, idx); } /* pop (FP, LR), restore SP to previous frame, return */ diff --git a/tcg/aarch64/tcg-target.h b/tcg/aarch64/tcg-target.h index 988983e..faccc36 100644 --- a/tcg/aarch64/tcg-target.h +++ b/tcg/aarch64/tcg-target.h @@ -17,17 +17,23 @@ #undef TCG_TARGET_STACK_GROWSUP typedef enum { -TCG_REG_X0, TCG_REG_X1, TCG_REG_X2, TCG_REG_X3, TCG_REG_X4, -TCG_REG_X5, TCG_REG_X6, TCG_REG_X7, TCG_REG_X8, TCG_REG_X9, -TCG_REG_X10, TCG_REG_X11, TCG_REG_X12, TCG_REG_X13, TCG_REG_X14, -TCG_REG_X15, TCG_REG_X16, TCG_REG_X17, TCG_REG_X18, TCG_REG_X19, -TCG_REG_X20, TCG_REG_X21, TCG_REG_X22, TCG_REG_X23, TCG_REG_X24, -TCG_REG_X25, TCG_REG_X26, TCG_REG_X27, TCG_REG_X28, -TCG_REG_FP, /* frame pointer */ -TCG_REG_LR, /* link register */ -TCG_REG_SP, /* stack pointer or zero register */ -TCG_REG_XZR = TCG_REG_SP /* same register number */ -/* program counter is not directly accessible! */ +TCG_REG_X0, TCG_REG_X1, TCG_REG_X2, TCG_REG_X3, +TCG_REG_X4, TCG_REG_X5, TCG_REG_X6,
Re: [Qemu-devel] [PATCH v3 4/4] gtk: Add Grab On Click option
On Fr, 2014-04-04 at 12:41 +0200, Takashi Iwai wrote: I simply like it better, you don't? :) I still think we should make this simply depend on absolute/relative pointer mode instead of asking the user to switch it manually. I'll quickly go put patches 1-3 into a 2.0 pull request, so debating patch #4 doesn't stop the clear improvements from entering the tree. cheers, Gerd
Re: [Qemu-devel] [Qemu-trivial] [PATCH v4] scripts: add sample model file for Coverity Scan
* Michael Tokarev (m...@tls.msk.ru) wrote: 26.03.2014 15:45, Paolo Bonzini wrote: This is the model file that is being used for the QEMU project's scans on scan.coverity.com. It fixed about 30 false positives (10% of the total) and exposed about 60 new memory leaks. The file is not automatically used; changes to it must be propagated to the website manually by an admin (right now Markus, Peter and me are admins). While we don't have issues with the model itself, but really wonder if we should keep this file inside qemu sources. It isn't used in there, as explicitly stated above, and maybe it is easier to maintain it closer to the website? Given that the model file describes semantics of functions in the codebase it needs to be kept consistent with the version of the code. Hence it needs to be version controlled with the code, hence the best place is inside the qemu sources. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH 03/97] vmstate: return error in case of error
* Juan Quintela (quint...@redhat.com) wrote: If there is an error while loading a field, we should stop reading and not continue with the rest of fields. Signed-off-by: Juan Quintela quint...@redhat.com Reviewed-by: Dr. David Alan Gilbert dgilb...@redhat.com --- vmstate.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vmstate.c b/vmstate.c index bfa34cc..d82cccf 100644 --- a/vmstate.c +++ b/vmstate.c @@ -74,6 +74,9 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd, ret = field-info-get(f, addr, size); } +if (ret = 0) { +ret = qemu_file_get_error(f); +} if (ret 0) { trace_vmstate_load_field_error(field-name, ret); return ret; -- 1.9.0 -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v3 4/4] gtk: Add Grab On Click option
At Mon, 07 Apr 2014 10:07:43 +0200, Gerd Hoffmann wrote: On Fr, 2014-04-04 at 12:41 +0200, Takashi Iwai wrote: I simply like it better, you don't? :) I still think we should make this simply depend on absolute/relative pointer mode instead of asking the user to switch it manually. Yes, agreed, it's more intuitive. OTOH, we still want the explicit input grab in absolute mode. So, we'll need two flags in the end, the explicit grab by menu or key combo, and the implicit grab by click in relative mode. I'll quickly go put patches 1-3 into a 2.0 pull request, so debating patch #4 doesn't stop the clear improvements from entering the tree. Thanks! Takashi
Re: [Qemu-devel] [PATCH 80/97] vmstate: Create VMSTATE_SYNTHETIC
* Juan Quintela (quint...@redhat.com) wrote: It is used for fields that don't exist on the State. They are generated on the fly for migration. While it's nicer than what's there before, I don't think this is the right fix for these fields, and I'd rather not encourage new uses like this. It still hides the type from the VMSTATE mechanism and ends up with the target-* code calling qemu_get*/qemu_put* on simple integers that VMSTATE does support. I was thinking something like: a) Set a .flags entry that this is a synthetic b) Change the .get/.put to post_load/pre_save c) Change the vmstate code to use a temporary if the synthetic flag is set, but then still call the pre_save/post_load on it passing the address of the temporary and of the real data somehow. That way the vmstate code still sees integers that it knows the type of, and we don't use qemu_get/qemu_put any more. Dave Signed-off-by: Juan Quintela quint...@redhat.com --- include/migration/vmstate.h | 14 ++ target-alpha/machine.c | 16 +++- target-arm/machine.c| 18 ++ 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index d695244..12020d9 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -201,6 +201,20 @@ extern const VMStateInfo vmstate_info_bitmap; .offset = vmstate_offset_value(_state, _field, _type), \ } +/* + * This is used for fields synthetized from the state, but that don't + * exist as such. That is the reaso of offset 0. They get the whole + * struct. + */ + +#define VMSTATE_SYNTHETIC(_name, _info, _size) { \ +.name = (_name), \ +.size = (_size), \ +.info = (_info),\ +.flags= VMS_SINGLE, \ +.offset = 0, \ +} + #define VMSTATE_POINTER(_field, _state, _test, _info, _type) { \ .name = (stringify(_field)), \ .info = (_info),\ diff --git a/target-alpha/machine.c b/target-alpha/machine.c index 5e69b1e..a5db209 100644 --- a/target-alpha/machine.c +++ b/target-alpha/machine.c @@ -23,20 +23,10 @@ static const VMStateInfo vmstate_fpcr = { static VMStateField vmstate_env_fields[] = { VMSTATE_UINTTL_ARRAY(ir, CPUAlphaState, 31), VMSTATE_UINTTL_ARRAY(fir, CPUAlphaState, 31), + /* Save the architecture value of the fpcr, not the internally - expanded version. Since this architecture value does not - exist in memory to be stored, this requires a but of hoop - jumping. We want OFFSET=0 so that we effectively pass ENV - to the helper functions, and we need to fill in the name by - hand since there's no field of that name. */ -{ -.name = fpcr, -.version_id = 0, -.size = sizeof(uint64_t), -.info = vmstate_fpcr, -.flags = VMS_SINGLE, -.offset = 0 -}, + expanded version. */ +VMSTATE_SYNTHETIC(fpcr, vmstate_fpcr, sizeof(uint64_t)), VMSTATE_UINTTL(pc, CPUAlphaState), VMSTATE_UINTTL(unique, CPUAlphaState), VMSTATE_UINTTL(lock_addr, CPUAlphaState), diff --git a/target-arm/machine.c b/target-arm/machine.c index 3f2c485..fd01e99 100644 --- a/target-arm/machine.c +++ b/target-arm/machine.c @@ -47,14 +47,7 @@ static const VMStateDescription vmstate_vfp = { */ VMSTATE_UINT32(env.vfp.xregs[0], ARMCPU), VMSTATE_UINT32_SUB_ARRAY(env.vfp.xregs, ARMCPU, 2, 14), -{ -.name = fpscr, -.version_id = 0, -.size = sizeof(uint32_t), -.info = vmstate_fpscr, -.flags = VMS_SINGLE, -.offset = 0, -}, +VMSTATE_SYNTHETIC(fpscr, vmstate_fpscr, sizeof(uint32)), VMSTATE_END_OF_LIST() } }; @@ -224,14 +217,7 @@ const VMStateDescription vmstate_arm_cpu = { .post_load = cpu_post_load, .fields = (VMStateField[]) { VMSTATE_UINT32_ARRAY(env.regs, ARMCPU, 16), -{ -.name = cpsr, -.version_id = 0, -.size = sizeof(uint32_t), -.info = vmstate_cpsr, -.flags = VMS_SINGLE, -.offset = 0, -}, +VMSTATE_SYNTHETIC(cpsr, vmstate_cpsr, sizeof(uint32_t)), VMSTATE_UINT32(env.spsr, ARMCPU), VMSTATE_UINT32_ARRAY(env.banked_spsr, ARMCPU, 6), VMSTATE_UINT32_ARRAY(env.banked_r13, ARMCPU, 6), -- 1.9.0 -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [Xen-devel] [PATCH 0/5] xen: add Intel IGD passthrough support
On Fri, 2014-04-04 at 18:46 -0400, Kevin O'Connor wrote: On Fri, Feb 21, 2014 at 02:44:08PM +0800, Yang Zhang wrote: From: Yang Zhang yang.z.zh...@intel.com The following patches are ported from Xen Qemu-traditional branch which are adding Intel IGD passthrough supporting to Qemu upstream. To pass through IGD to guest, user need to add following lines in Xen config file: gfx_passthru=1 pci=['00:02.0@2'] Besides, since Xen + Qemu upstream is requiring seabios, user also need to recompile seabios with CONFIG_OPTIONROMS_DEPLOYED=y to allow IGD pass through successfully: I'm not familiar with the Xen parts of your patch series. However, I don't think one should be compiling SeaBIOS with CONFIG_OPTIONROMS_DEPLOYED enabled. I agree, thanks for spotting this. I would nack any patch which tried to enable this option in the Xen build of SeaBIOS. That option was for very old versions of Bochs and QEMU that did not support fetching of optionroms directly from PCI config space nor from fw_cfg. That compile time option is no longer well supported and it may be removed in the future. I suggest looking at getting one of the other mechanisms working instead of using CONFIG_OPTIONROMS_DEPLOYED. AFAIK the usual mechanisms already work for SeaBIOS under Xen, e.g. for the video BIOS of the emulated VGA it Just Works. I've no idea what makes Intel IGD passthrough special. Ian.
Re: [Qemu-devel] [PATCH for-2.0] dsdt: tweak ACPI ID for hotplug resource device
On Mon, Apr 07, 2014 at 09:46:34AM +0200, Igor Mammedov wrote: On Sun, 6 Apr 2014 12:47:37 +0300 Michael S. Tsirkin m...@redhat.com wrote: ACPI0004 seems too new: Windows XP complains about an unrecognized device. This is a regression since 1.7. Use PNP0A06 instead - Generic Container Device. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Note: Igor has RFC patches to use PNP0C02 under PCI0, but that's not ready for 2.0. Igor, could you comment on whether PNP0C02 is preferable for some OS-es? For PNP0A06 ACPI spec says: PNP0A06 Generic Container Device. This device behaves exactly the same as the PNP0A05 device. This was originally known as Extended I/O Bus. This ID should only be used for containers that do not produce resources for consumption by child devices. Any system resources claimed by a PNP0A06 device’s _CRS object must be consumed by the container itself. PNP0C02 is not in ACPI spec. It does appear in MicroSoft legacy PNP ID document: PNP0C02 General ID for reserving resources required by Plug and Play motherboard registers. (Not specific to a particular device.) Thoughts? From testing Windows doesn't check for conflicts _CRS provided by PNP0C02, so while using it will let XP not to complain about unknown device, it will also make later Windows OSes silently ignore conflicts if any. When XP's complaining for the first time about unknown device, it could be told to ignore it, so it would stop to complain in future. i.e. user have to silence only once. Wouldn't it be better to use ACPI0004 that gives minor annoyance to XP user but provides error checking with later OS versions? OK I quickly tested with Win7 and it does check for conflicts when EisaId(PNP0A06) is used. Taking this into account, ack this patch? hw/i386/acpi-dsdt-cpu-hotplug.dsl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl index dee4843..34aab5a 100644 --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl @@ -93,7 +93,7 @@ Scope(\_SB) { } Device(CPU_HOTPLUG_RESOURCE_DEVICE) { -Name(_HID, ACPI0004) +Name(_HID, EisaId(PNP0A06)) Name(_CRS, ResourceTemplate() { IO(Decode16, CPU_STATUS_BASE, CPU_STATUS_BASE, 0, CPU_STATUS_LEN)
Re: [Qemu-devel] [PATCH 96/97] vmstate: Rename VMS_VBUFFER to VMST_VBUFFER_INT32 for consintency
* Juan Quintela (quint...@redhat.com) wrote: Signed-off-by: Juan Quintela quint...@redhat.com --- include/migration/vmstate.h | 6 +++--- vmstate.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 976d83e..145c198 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -96,7 +96,7 @@ enum VMStateFlags { VMS_BUFFER = 0x020, /* static sized buffer */ VMS_ARRAY_OF_POINTER = 0x040, VMS_VARRAY_UINT16= 0x080, /* Array with size in uint16_t field */ -VMS_VBUFFER = 0x100, /* Buffer with size in int32_t field */ +VMS_VBUFFER_INT32= 0x100, /* Buffer with size in int32_t field */ VMS_VBUFFER_UINT32 = 0x200, /* Buffer with size in uint32_t field */ VMS_VARRAY_UINT8 = 0x400, /* Array with size in uint8_t field*/ VMS_VARRAY_UINT32= 0x800, /* Array with size in uint32_t field*/ @@ -436,7 +436,7 @@ extern const VMStateInfo vmstate_info_bitmap; .name = (stringify(_field)), \ .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\ .info = vmstate_info_buffer,\ -.flags= VMS_VBUFFER|VMS_POINTER, \ +.flags= VMS_VBUFFER_INT32|VMS_POINTER, \ Are there any real users of the code where it's really an INT32 value - can we just not kill off the silly idea of signed buffer sizes altogether? (The only case I can follow through is onenand.c that has a PARTIAL_VBUFFER based on an 'int' called 'blocks', that's derived during init from an unsigned value and passed to things like mallocs and memsets, so it's never going to be negative). Dave .offset = offsetof(_state, _field),\ } @@ -480,7 +480,7 @@ extern const VMStateInfo vmstate_info_bitmap; .name = (stringify(_field)), \ .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\ .info = vmstate_info_bitmap,\ -.flags= VMS_VBUFFER|VMS_POINTER, \ +.flags= VMS_VBUFFER_INT32|VMS_POINTER, \ .offset = offsetof(_state, _field),\ } diff --git a/vmstate.c b/vmstate.c index da5c49d..b1ff280 100644 --- a/vmstate.c +++ b/vmstate.c @@ -33,7 +33,7 @@ static int vmstate_size(void *opaque, VMStateField *field) { int size = field-size; -if (field-flags VMS_VBUFFER) { +if (field-flags VMS_VBUFFER_INT32) { size = *(int32_t *)(opaque+field-size_offset); } else if (field-flags VMS_VBUFFER_UINT32) { size = *(uint32_t *)(opaque+field-size_offset); -- 1.9.0 -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] [PULL for-2.0 0/4] gtk: pointer fixes
Hi, Last minute pointer fixes for gtk. And a MAINTAINERS update to help ui patches being picked up more timely in the future. please pull, Gerd The following changes since commit 466e6e9d13d56bbb6da1d2396d7d6347df483af0: target-i386: reorder fields in cpu/msr_hyperv_hypercall subsection (2014-04-05 10:49:05 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-gtk-4 for you to fetch changes up to 25eccc37ff91254efdd123f5dafb37526a83a990: ui: Update MAINTAINERS entry. (2014-04-07 10:50:30 +0200) gtk: pointer fixes from Takashi Iwai. Gerd Hoffmann (1): ui: Update MAINTAINERS entry. Takashi Iwai (3): gtk: Use gtk generic event signal instead of motion-notify-event gtk: Fix the relative pointer tracking mode gtk: Remember the last grabbed pointer position MAINTAINERS | 3 ++- ui/gtk.c| 46 +- 2 files changed, 35 insertions(+), 14 deletions(-)
[Qemu-devel] [PULL 4/4] ui: Update MAINTAINERS entry.
With Amazon eating Anthonys time status Maintained certainly isn't true any more. Update entry accordingly. Also add myself, so scripts/get_maintainer.pl will Cc: me, to reduce the chance ui patches fall through the cracks on our pretty loaded qemu-devel mailing list. Signed-off-by: Gerd Hoffmann kra...@redhat.com --- MAINTAINERS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 34b8c3f..c66946f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -717,7 +717,8 @@ F: hw/display/qxl* Graphics M: Anthony Liguori aligu...@amazon.com -S: Maintained +M: Gerd Hoffmann kra...@redhat.com +S: Odd Fixes F: ui/ Cocoa graphics -- 1.8.3.1
[Qemu-devel] [PULL 1/4] gtk: Use gtk generic event signal instead of motion-notify-event
From: Takashi Iwai ti...@suse.de The GDK motion-notify-event isn't generated when the pointer goes out of the target window even if the pointer is grabbed, which essentially means to lose the pointer tracking in gtk-ui. Meanwhile the generic event signal is sent when the pointer is grabbed, so we can use this and pick the motion notify events manually there instead. Reference: https://bugzilla.novell.com/show_bug.cgi?id=849587 Tested-by: Cole Robinson crobi...@redhat.com Reviewed-by: Cole Robinson crobi...@redhat.com Tested-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Takashi Iwai ti...@suse.de Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/gtk.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index f056e40..c9f6d24 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -765,6 +765,14 @@ static gboolean gd_key_event(GtkWidget *widget, GdkEventKey *key, void *opaque) return TRUE; } +static gboolean gd_event(GtkWidget *widget, GdkEvent *event, void *opaque) +{ +if (event-type == GDK_MOTION_NOTIFY) { +return gd_motion_event(widget, event-motion, opaque); +} +return FALSE; +} + /** Window Menu Actions **/ static void gd_menu_pause(GtkMenuItem *item, void *opaque) @@ -1267,8 +1275,8 @@ static void gd_connect_signals(GtkDisplayState *s) g_signal_connect(s-drawing_area, expose-event, G_CALLBACK(gd_expose_event), s); #endif -g_signal_connect(s-drawing_area, motion-notify-event, - G_CALLBACK(gd_motion_event), s); +g_signal_connect(s-drawing_area, event, + G_CALLBACK(gd_event), s); g_signal_connect(s-drawing_area, button-press-event, G_CALLBACK(gd_button_event), s); g_signal_connect(s-drawing_area, button-release-event, -- 1.8.3.1
[Qemu-devel] [PULL 2/4] gtk: Fix the relative pointer tracking mode
From: Takashi Iwai ti...@suse.de The relative pointer tracking mode was still buggy even after the previous fix of the motion-notify-event since the events are filtered out when the pointer moves outside the drawing window due to the boundary check for the absolute mode. This patch fixes the issue by moving the unnecessary boundary check into the if block of absolute mode, and keep the coordinate in the relative mode even if it's outside the drawing area. But this makes the coordinate (last_x, last_y) possibly pointing to (-1,-1), introduce a new flag to indicate the last coordinate has been updated. Reference: https://bugzilla.novell.com/show_bug.cgi?id=849587 Tested-by: Cole Robinson crobi...@redhat.com Reviewed-by: Cole Robinson crobi...@redhat.com Tested-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Takashi Iwai ti...@suse.de Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/gtk.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index c9f6d24..913cc3f 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -156,6 +156,7 @@ typedef struct GtkDisplayState DisplayChangeListener dcl; DisplaySurface *ds; int button_mask; +gboolean last_set; int last_x; int last_y; @@ -616,25 +617,25 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion, x = (motion-x - mx) / s-scale_x; y = (motion-y - my) / s-scale_y; -if (x 0 || y 0 || -x = surface_width(s-ds) || -y = surface_height(s-ds)) { -return TRUE; -} - if (qemu_input_is_absolute()) { +if (x 0 || y 0 || +x = surface_width(s-ds) || +y = surface_height(s-ds)) { +return TRUE; +} qemu_input_queue_abs(s-dcl.con, INPUT_AXIS_X, x, surface_width(s-ds)); qemu_input_queue_abs(s-dcl.con, INPUT_AXIS_Y, y, surface_height(s-ds)); qemu_input_event_sync(); -} else if (s-last_x != -1 s-last_y != -1 gd_is_grab_active(s)) { +} else if (s-last_set gd_is_grab_active(s)) { qemu_input_queue_rel(s-dcl.con, INPUT_AXIS_X, x - s-last_x); qemu_input_queue_rel(s-dcl.con, INPUT_AXIS_Y, y - s-last_y); qemu_input_event_sync(); } s-last_x = x; s-last_y = y; +s-last_set = TRUE; if (!qemu_input_is_absolute() gd_is_grab_active(s)) { GdkScreen *screen = gtk_widget_get_screen(s-drawing_area); @@ -669,8 +670,7 @@ static gboolean gd_motion_event(GtkWidget *widget, GdkEventMotion *motion, GdkDisplay *display = gtk_widget_get_display(widget); gdk_display_warp_pointer(display, screen, x, y); #endif -s-last_x = -1; -s-last_y = -1; +s-last_set = FALSE; return FALSE; } } -- 1.8.3.1
Re: [Qemu-devel] [PATCH for-2.0] dsdt: tweak ACPI ID for hotplug resource device
On Mon, Apr 07, 2014 at 10:57:02AM +0200, Igor Mammedov wrote: On Sun, 6 Apr 2014 12:47:37 +0300 Michael S. Tsirkin m...@redhat.com wrote: ACPI0004 seems too new: Windows XP complains about an unrecognized device. This is a regression since 1.7. Use PNP0A06 instead - Generic Container Device. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Note: Igor has RFC patches to use PNP0C02 under PCI0, but that's not ready for 2.0. Igor, could you comment on whether PNP0C02 is preferable for some OS-es? For PNP0A06 ACPI spec says: PNP0A06 Generic Container Device. This device behaves exactly the same as the PNP0A05 device. This was originally known as Extended I/O Bus. This ID should only be used for containers that do not produce resources for consumption by child devices. Any system resources claimed by a PNP0A06 device’s _CRS object must be consumed by the container itself. PNP0C02 is not in ACPI spec. It does appear in MicroSoft legacy PNP ID document: PNP0C02 General ID for reserving resources required by Plug and Play motherboard registers. (Not specific to a particular device.) Thoughts? hw/i386/acpi-dsdt-cpu-hotplug.dsl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl index dee4843..34aab5a 100644 --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl @@ -93,7 +93,7 @@ Scope(\_SB) { } Device(CPU_HOTPLUG_RESOURCE_DEVICE) { -Name(_HID, ACPI0004) +Name(_HID, EisaId(PNP0A06)) Name(_CRS, ResourceTemplate() { IO(Decode16, CPU_STATUS_BASE, CPU_STATUS_BASE, 0, CPU_STATUS_LEN) Reviewed-By: Igor Mammedov imamm...@redhat.com Thanks! Peter, could you pick this up for 2.0 please? -- MST
[Qemu-devel] [PULL 3/4] gtk: Remember the last grabbed pointer position
From: Takashi Iwai ti...@suse.de It's pretty annoying that the pointer reappears at a random place once after grabbing and ungrabbing the input. Better to restore to the original position where the pointer was grabbed. Reference: https://bugzilla.novell.com/show_bug.cgi?id=849587 Tested-by: Cole Robinson crobi...@redhat.com Reviewed-by: Cole Robinson crobi...@redhat.com Tested-by: Michael S. Tsirkin m...@redhat.com Signed-off-by: Takashi Iwai ti...@suse.de Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/gtk.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/ui/gtk.c b/ui/gtk.c index 913cc3f..6668bd8 100644 --- a/ui/gtk.c +++ b/ui/gtk.c @@ -159,6 +159,8 @@ typedef struct GtkDisplayState gboolean last_set; int last_x; int last_y; +int grab_x_root; +int grab_y_root; double scale_x; double scale_y; @@ -971,8 +973,8 @@ static void gd_ungrab_keyboard(GtkDisplayState *s) static void gd_grab_pointer(GtkDisplayState *s) { -#if GTK_CHECK_VERSION(3, 0, 0) GdkDisplay *display = gtk_widget_get_display(s-drawing_area); +#if GTK_CHECK_VERSION(3, 0, 0) GdkDeviceManager *mgr = gdk_display_get_device_manager(display); GList *devices = gdk_device_manager_list_devices(mgr, GDK_DEVICE_TYPE_MASTER); @@ -996,6 +998,8 @@ static void gd_grab_pointer(GtkDisplayState *s) tmp = tmp-next; } g_list_free(devices); +gdk_device_get_position(gdk_device_manager_get_client_pointer(mgr), +NULL, s-grab_x_root, s-grab_y_root); #else gdk_pointer_grab(gtk_widget_get_window(s-drawing_area), FALSE, /* All events to come to our window directly */ @@ -1007,13 +1011,15 @@ static void gd_grab_pointer(GtkDisplayState *s) NULL, /* Allow cursor to move over entire desktop */ s-null_cursor, GDK_CURRENT_TIME); +gdk_display_get_pointer(display, NULL, +s-grab_x_root, s-grab_y_root, NULL); #endif } static void gd_ungrab_pointer(GtkDisplayState *s) { -#if GTK_CHECK_VERSION(3, 0, 0) GdkDisplay *display = gtk_widget_get_display(s-drawing_area); +#if GTK_CHECK_VERSION(3, 0, 0) GdkDeviceManager *mgr = gdk_display_get_device_manager(display); GList *devices = gdk_device_manager_list_devices(mgr, GDK_DEVICE_TYPE_MASTER); @@ -1027,8 +1033,14 @@ static void gd_ungrab_pointer(GtkDisplayState *s) tmp = tmp-next; } g_list_free(devices); +gdk_device_warp(gdk_device_manager_get_client_pointer(mgr), +gtk_widget_get_screen(s-drawing_area), +s-grab_x_root, s-grab_y_root); #else gdk_pointer_ungrab(GDK_CURRENT_TIME); +gdk_display_warp_pointer(display, + gtk_widget_get_screen(s-drawing_area), + s-grab_x_root, s-grab_y_root); #endif } -- 1.8.3.1
Re: [Qemu-devel] [PATCH for-2.0] dsdt: tweak ACPI ID for hotplug resource device
On Sun, 6 Apr 2014 12:47:37 +0300 Michael S. Tsirkin m...@redhat.com wrote: ACPI0004 seems too new: Windows XP complains about an unrecognized device. This is a regression since 1.7. Use PNP0A06 instead - Generic Container Device. Signed-off-by: Michael S. Tsirkin m...@redhat.com --- Note: Igor has RFC patches to use PNP0C02 under PCI0, but that's not ready for 2.0. Igor, could you comment on whether PNP0C02 is preferable for some OS-es? For PNP0A06 ACPI spec says: PNP0A06 Generic Container Device. This device behaves exactly the same as the PNP0A05 device. This was originally known as Extended I/O Bus. This ID should only be used for containers that do not produce resources for consumption by child devices. Any system resources claimed by a PNP0A06 device’s _CRS object must be consumed by the container itself. PNP0C02 is not in ACPI spec. It does appear in MicroSoft legacy PNP ID document: PNP0C02 General ID for reserving resources required by Plug and Play motherboard registers. (Not specific to a particular device.) Thoughts? hw/i386/acpi-dsdt-cpu-hotplug.dsl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/i386/acpi-dsdt-cpu-hotplug.dsl b/hw/i386/acpi-dsdt-cpu-hotplug.dsl index dee4843..34aab5a 100644 --- a/hw/i386/acpi-dsdt-cpu-hotplug.dsl +++ b/hw/i386/acpi-dsdt-cpu-hotplug.dsl @@ -93,7 +93,7 @@ Scope(\_SB) { } Device(CPU_HOTPLUG_RESOURCE_DEVICE) { -Name(_HID, ACPI0004) +Name(_HID, EisaId(PNP0A06)) Name(_CRS, ResourceTemplate() { IO(Decode16, CPU_STATUS_BASE, CPU_STATUS_BASE, 0, CPU_STATUS_LEN) Reviewed-By: Igor Mammedov imamm...@redhat.com
Re: [Qemu-devel] [Qemu-trivial] [PATCH v4] scripts: add sample model file for Coverity Scan
26.03.2014 15:45, Paolo Bonzini wrote: This is the model file that is being used for the QEMU project's scans on scan.coverity.com. It fixed about 30 false positives (10% of the total) and exposed about 60 new memory leaks. The file is not automatically used; changes to it must be propagated to the website manually by an admin (right now Markus, Peter and me are admins). Applied to -trivial, thank you! Note: I haven't actually checked this file for correctness myself ;) /mjt
Re: [Qemu-devel] [Qemu-trivial] [PATCH] configure: Remove redundant message for -Werror
15.03.2014 00:11, Stefan Weil wrote: The compiler flag -Werror is printed (or not printed) as any other compiler flag which is part of QEMU_CFLAGS. Therefore an extra output line for -Werror is redundant and can be removed. Applied - finally - to -trivial, thank you! /mjt
Re: [Qemu-devel] [PATCH for 2.1 00/97] VMState simplification (massive)
* Juan Quintela (quint...@redhat.com) wrote: Hi Look at the diffstat. Almost all the additions are at test-vmstate.c. That is the reason why it is called a simplification. What this series does: - peter removal of version_minimum_id_old field when not needed (Peter) - cleanup: based on the previous one, I removed all the unneeded the uses on the tree. This should make your compiles a couple of nanoseconds faster. - once there, fixed the indentation of the .fields line, to a canonical .fields = (VMStateField[]) - mst simplifications for vmstate engine And now, the big cleanup. - Patches only do one thing, to make easy the review. - Added test for all VMSTATE_FOO() definitions (well, I am lying, VMSTATE_STRUCT* are still missing, will come soon) - We had two ways to make a field optional VMSTATE_INT64_V(field, state, version) and VMSTATE_INT64_TEST(field, state, test) We can do the version one with one test like: static inline bool vmstate_5_plus(void *opaque, int version_id) { return version_id = 5; } and then change: VMSTATE_INT64_V(field, state, 5); into VMSTATE_INT64_TEST(field, state, vmstate_5_plus); I'm not sure if I like this; while I'm OK with the idea of changing the implementation of VMSTATE_INT64_V to use that function trick internally, it seems like we're discouraging providing easy to parse/record versionining info out of the tree. Dave -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] [PATCH] doc: grammify allows to
English language grammar does not allow usage of the word allows directly followed by an infinitive, declaring constructs like something allows to do somestuff un-grammatical. Often it is possible to just insert one between allows and to to make the construct grammatical, but usually it is better to re-phrase the statement. This patch tries to fix 2 examples of allows to usage in qemu doc, but does not address comments in the code with similar constructs. In qemu-options.hx this patch also adds a missing the on the same line. Signed-off-by: Michael Tokarev m...@tls.msk.ru --- qemu-doc.texi |2 +- qemu-options.hx |3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/qemu-doc.texi b/qemu-doc.texi index e6e20eb..88ec9bb 100644 --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -823,7 +823,7 @@ In this case, the block device must be exported using qemu-nbd: qemu-nbd --socket=/tmp/my_socket my_disk.qcow2 @end example -The use of qemu-nbd allows to share a disk between several guests: +The use of qemu-nbd allows sharing of a disk between several guests: @example qemu-nbd --socket=/tmp/my_socket --share=2 my_disk.qcow2 @end example diff --git a/qemu-options.hx b/qemu-options.hx index 2d33815..585cb8d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -444,7 +444,8 @@ This option defines the type of the media: disk or cdrom. @item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}] These options have the same definition as they have in @option{-hdachs}. @item snapshot=@var{snapshot} -@var{snapshot} is on or off and allows to enable snapshot for given drive (see @option{-snapshot}). +@var{snapshot} is on or off and controls snapshot mode for the given drive +(see @option{-snapshot}). @item cache=@var{cache} @var{cache} is none, writeback, unsafe, directsync or writethrough and controls how the host cache is used to access block data. @item aio=@var{aio} -- 1.7.10.4
Re: [Qemu-devel] [PATCH v3 11/26] tcg-aarch64: Reuse LR in translated code
On 7 April 2014 09:03, Claudio Fontana claudio.font...@huawei.com wrote: On 03.04.2014 21:56, Richard Henderson wrote: It's obviously call-clobbered, but is otherwise unused. Repurpose it as the TCG temporary. Giving one last chance to the ARM guys to speak up about repurposing LR. Can you clarify what you think the issue is with using LR? I think you said before but I forget :-( thanks -- PMM
Re: [Qemu-devel] [PATCH 96/97] vmstate: Rename VMS_VBUFFER to VMST_VBUFFER_INT32 for consintency
Dr. David Alan Gilbert dgilb...@redhat.com wrote: * Juan Quintela (quint...@redhat.com) wrote: Signed-off-by: Juan Quintela quint...@redhat.com --- include/migration/vmstate.h | 6 +++--- vmstate.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 976d83e..145c198 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -96,7 +96,7 @@ enum VMStateFlags { VMS_BUFFER = 0x020, /* static sized buffer */ VMS_ARRAY_OF_POINTER = 0x040, VMS_VARRAY_UINT16= 0x080, /* Array with size in uint16_t field */ -VMS_VBUFFER = 0x100, /* Buffer with size in int32_t field */ +VMS_VBUFFER_INT32= 0x100, /* Buffer with size in int32_t field */ VMS_VBUFFER_UINT32 = 0x200, /* Buffer with size in uint32_t field */ VMS_VARRAY_UINT8 = 0x400, /* Array with size in uint8_t field*/ VMS_VARRAY_UINT32= 0x800, /* Array with size in uint32_t field*/ @@ -436,7 +436,7 @@ extern const VMStateInfo vmstate_info_bitmap; .name = (stringify(_field)), \ .size_offset = vmstate_offset_value(_state, _field_size, int32_t),\ .info = vmstate_info_buffer,\ -.flags= VMS_VBUFFER|VMS_POINTER, \ +.flags= VMS_VBUFFER_INT32|VMS_POINTER, \ Are there any real users of the code where it's really an INT32 value - can we just not kill off the silly idea of signed buffer sizes altogether? (The only case I can follow through is onenand.c that has a PARTIAL_VBUFFER based on an 'int' called 'blocks', that's derived during init from an unsigned value and passed to things like mallocs and memsets, so it's never going to be negative). There is one user :-( Note the VARRAY thing, where there are more. And normally it depends on the target device if it is signed or not Later, Juan.
Re: [Qemu-devel] [PATCH 80/97] vmstate: Create VMSTATE_SYNTHETIC
Dr. David Alan Gilbert dgilb...@redhat.com wrote: * Juan Quintela (quint...@redhat.com) wrote: It is used for fields that don't exist on the State. They are generated on the fly for migration. While it's nicer than what's there before, I don't think this is the right fix for these fields, and I'd rather not encourage new uses like this. It still hides the type from the VMSTATE mechanism and ends up with the target-* code calling qemu_get*/qemu_put* on simple integers that VMSTATE does support. I was thinking something like: a) Set a .flags entry that this is a synthetic b) Change the .get/.put to post_load/pre_save c) Change the vmstate code to use a temporary if the synthetic flag is set, but then still call the pre_save/post_load on it passing the address of the temporary and of the real data somehow. That way the vmstate code still sees integers that it knows the type of, and we don't use qemu_get/qemu_put any more. That is what I proposed on the introduction email. That would also solve mst VMS_VALIDATE plroblems. Later, Juan.
Re: [Qemu-devel] [Qemu-trivial] [PATCH v4] scripts: add sample model file for Coverity Scan
Michael Tokarev m...@tls.msk.ru writes: 26.03.2014 15:45, Paolo Bonzini wrote: This is the model file that is being used for the QEMU project's scans on scan.coverity.com. It fixed about 30 false positives (10% of the total) and exposed about 60 new memory leaks. The file is not automatically used; changes to it must be propagated to the website manually by an admin (right now Markus, Peter and me are admins). Applied to -trivial, thank you! Note: I haven't actually checked this file for correctness myself ;) I checked v2. I suggested a few minor improvements, and some of them made it into this version. Since Paolo's initial version comes in part from me, I guess it would be proper to add Signed-off-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] [Xen-devel] Qemu 2.0 regression with xen: qemu crash on any domUs S.O. start
Il 03/04/2014 12:13, Fabio Fantoni ha scritto: Il 03/04/2014 10:45, Ian Campbell ha scritto: On Thu, 2014-04-03 at 10:15 +0200, Fabio Fantoni wrote: Seems that do segfault when I connect to vnc or spice, in the test of this backtrace after connect to vnc, spice and other things of my patches are disabled, so do not think it is a problem caused by my patches. The last spice patch of yours I saw was incorrectly accessing the wrong half of various unions which is liable to cause all sorts of corruption or strange behaviour. Please can you reproduce this issue without any patches applied. Ian. After saw the full backtrace I saw on qemu git recent patches with fix on input, than I tried to update qemu to latest commit (82c6f513735297ad76acaaf2e87f0c5a0b3647a7) and now the segfault seems solve, I did some fast test with vnc and spice on same pv domUs without qemu crashes. About libxl patch of spice support for pv domUs I'll improve it following your reply and also try to find more details about pointer not visible but working with spice on pv domUs. Thanks to all for your help. Today I did some tests also with hvm and spice and I found another segfault with different backtrace to solve: (gdb) c Continuing. *Program received signal SIGSEGV, Segmentation fault.** **0x55855d30 in interface_client_monitors_config (sin=0x563b0260, ** **mc=0x0) at ui/spice-display.c:557** **557 if (mc-num_of_monitors 0) {* (gdb) bt full #0 0x55855d30 in interface_client_monitors_config ( sin=0x563b0260, mc=0x0) at ui/spice-display.c:557 ssd = 0x563b0210 info = {xoff = 0, yoff = 0, width = 0, height = 0} rc = 32767 __func__ = interface_client_monitors_config #1 0x74af5113 in ?? () from /usr/lib/x86_64-linux-gnu/libspice-server.so.1 No symbol table info available. #2 0x74ad87f5 in ?? () from /usr/lib/x86_64-linux-gnu/libspice-server.so.1 No symbol table info available. #3 0x74b1af76 in ?? () from /usr/lib/x86_64-linux-gnu/libspice-server.so.1 No symbol table info available. #4 0x74ae989a in ?? () from /usr/lib/x86_64-linux-gnu/libspice-server.so.1 No symbol table info available. #5 0x74aee470 in ?? () from /usr/lib/x86_64-linux-gnu/libspice-server.so.1 No symbol table info available. #6 0x74af0d8c in ?? () from /usr/lib/x86_64-linux-gnu/libspice-server.so.1 No symbol table info available. #7 0x55851f82 in watch_read (opaque=0x5666a8d0) ---Type return to continue, or q return to quit--- at ui/spice-core.c:101 watch = 0x5666a8d0 #8 0x557ce1f8 in qemu_iohandler_poll (pollfds=0x562e8e00, ret=2) at iohandler.c:143 revents = 1 pioh = 0x5634e080 ioh = 0x5666adb0 #9 0x557cf2a4 in main_loop_wait (nonblocking=0) at main-loop.c:485 ret = 2 timeout = 4294967295 timeout_ns = 25664603 #10 0x5587acd8 in main_loop () at vl.c:2051 nonblocking = false last_io = 3 #11 0x558826b2 in main (argc=36, argv=0x7fffe368, envp=0x7fffe490) at vl.c:4507 i = 64 snapshot = 0 linux_boot = 0 icount_option = 0x0 initrd_filename = 0x0 kernel_filename = 0x0 kernel_cmdline = 0x55a1b5c4 boot_order = 0x562e7ee0 dc ds = 0x563d8fd0 ---Type return to continue, or q return to quit--- cyls = 0 heads = 0 secs = 0 translation = 0 hda_opts = 0x0 opts = 0x562e7e30 machine_opts = 0x562e84b0 olist = 0x55e00e00 optind = 36 optarg = 0x7fffe923 if=ide,index=1,media=cdrom,cache=writeback,id=ide-832 loadvm = 0x0 machine_class = 0x562e02a0 machine = 0x55e067e0 cpu_model = 0x0 vga_model = 0x0 qtest_chrdev = 0x0 qtest_log = 0x0 pid_file = 0x0 incoming = 0x0 show_vnc_port = 0 defconfig = true userconfig = true log_mask = 0x0 log_file = 0x0 ---Type return to continue, or q return to quit--- mem_trace = {malloc = 0x5587e56a malloc_and_trace, realloc = 0x5587e5c2 realloc_and_trace, free = 0x5587e629 free_and_trace, calloc = 0, try_malloc = 0, try_realloc = 0} trace_events = 0x0 trace_file = 0x0 __func__ = main args = {machine = 0x55e067e0, ram_size = 2130706432, boot_order = 0x562e7ee0 dc, kernel_filename = 0x0, kernel_cmdline = 0x55a1b5c4 , initrd_filename = 0x0, cpu_model = 0x0} (gdb) qemu from source git/master commit 82c6f513735297ad76acaaf2e87f0c5a0b3647a7 spice server packages is version 0.12.4-0nocelt2 recompiled from debian unstable source. If you need more informations/tests tell me and I'll post them. Thanks for any reply.
[Qemu-devel] [PATCH trivial v2] doc: grammify allows to
English language grammar does not allow usage of the word allows directly followed by an infinitive, declaring constructs like something allows to do somestuff un-grammatical. Often it is possible to just insert one between allows and to to make the construct grammatical, but usually it is better to re-phrase the statement. This patch tries to fix 3 examples of allows to usage in qemu doc, but does not address comments in the code with similar constructs. It also adds missing the in the same line. Signed-off-by: Michael Tokarev m...@tls.msk.ru --- v2: catch one more occurence of the same construct in qemu-options.hx, in vnc encodings section. This statement may need rewriting which I don't provide in this patch, which just fixes the grammar. qemu-doc.texi |2 +- qemu-options.hx |5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/qemu-doc.texi b/qemu-doc.texi index e6e20eb..88ec9bb 100644 --- a/qemu-doc.texi +++ b/qemu-doc.texi @@ -823,7 +823,7 @@ In this case, the block device must be exported using qemu-nbd: qemu-nbd --socket=/tmp/my_socket my_disk.qcow2 @end example -The use of qemu-nbd allows to share a disk between several guests: +The use of qemu-nbd allows sharing of a disk between several guests: @example qemu-nbd --socket=/tmp/my_socket --share=2 my_disk.qcow2 @end example diff --git a/qemu-options.hx b/qemu-options.hx index 2d33815..3a7cc8f 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -444,7 +444,8 @@ This option defines the type of the media: disk or cdrom. @item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}] These options have the same definition as they have in @option{-hdachs}. @item snapshot=@var{snapshot} -@var{snapshot} is on or off and allows to enable snapshot for given drive (see @option{-snapshot}). +@var{snapshot} is on or off and controls snapshot mode for the given drive +(see @option{-snapshot}). @item cache=@var{cache} @var{cache} is none, writeback, unsafe, directsync or writethrough and controls how the host cache is used to access block data. @item aio=@var{aio} @@ -1242,7 +1243,7 @@ Disable adaptive encodings. Adaptive encodings are enabled by default. An adaptive encoding will try to detect frequently updated screen regions, and send updates in these regions using a lossy encoding (like JPEG). This can be really helpful to save bandwidth when playing videos. Disabling -adaptive encodings allows to restore the original static behavior of encodings +adaptive encodings restores the original static behavior of encodings like Tight. @item share=[allow-exclusive|force-shared|ignore] -- 1.7.10.4
Re: [Qemu-devel] [PATCH for 2.1 00/97] VMState simplification (massive)
Dr. David Alan Gilbert dgilb...@redhat.com wrote: * Juan Quintela (quint...@redhat.com) wrote: Hi Look at the diffstat. Almost all the additions are at test-vmstate.c. That is the reason why it is called a simplification. What this series does: - peter removal of version_minimum_id_old field when not needed (Peter) - cleanup: based on the previous one, I removed all the unneeded the uses on the tree. This should make your compiles a couple of nanoseconds faster. - once there, fixed the indentation of the .fields line, to a canonical .fields = (VMStateField[]) - mst simplifications for vmstate engine And now, the big cleanup. - Patches only do one thing, to make easy the review. - Added test for all VMSTATE_FOO() definitions (well, I am lying, VMSTATE_STRUCT* are still missing, will come soon) - We had two ways to make a field optional VMSTATE_INT64_V(field, state, version) and VMSTATE_INT64_TEST(field, state, test) We can do the version one with one test like: static inline bool vmstate_5_plus(void *opaque, int version_id) { return version_id = 5; } and then change: VMSTATE_INT64_V(field, state, 5); into VMSTATE_INT64_TEST(field, state, vmstate_5_plus); I'm not sure if I like this; while I'm OK with the idea of changing the implementation of VMSTATE_INT64_V to use that function trick internally, it seems like we're discouraging providing easy to parse/record versionining info out of the tree. That information is not exported Today. And if we want to export them, export v = 5 or test == vmstat_5_plus is exactly the same dificulty. The information is still there, what has changed is the removal of one mechanism. #define VMSTATE_UINT64_V(field, state, X) \ VMSTATE_UINT64_TEST(field, state, vmstate_##X##_plus) The same that we could do for maintaining the macro, we can do to export the information to whatever format we want. I just wanted to remove one of the mechanism (the less powerful one). Later, Juan.
Re: [Qemu-devel] Bug#728876: qemu: smbd forked by qemu uses global directory /var/run/samba/ncalrpc
05.02.2014 13:49, Michael Tokarev wrote: Ping? Again, more than 2 months passed since initial submission. Ping#2 ? Should it go to -trivial maybe? Thanks, /mjt 29.11.2013 00:15, Michael Tokarev wrote: Jan, there's one more samba-related fix for slirp, also from Michael Büsch. Add my Signed-off-By: Michael Tokarev m...@tls.msk.ru if needed. Thanks, /mjt 06.11.2013 17:01, Michael Büsch wrote: Package: qemu Version: 1.6.0+dfsg-2 Severity: normal Tags: patch The smbd forked by qemu still uses the default ncalrpc directory in /var/run/samba. This may lead to problems, if the directory does not exist (for example if /var/run is a tmpfs and the host smbd was not started). This leads to the following error message from samba and an unworkable smbd: Failed to create pipe directory /var/run/samba/ncalrpc - No such file or directory The attached patch fixes this by pointing smbd to /tmp/qemu-smb.%d.%d/ncalrpc as ncalrpc directory. Smbd will create the actual ncalrpc subdirectory on its own. Using a private directory also avoids possible clashes with the system-smbd.
Re: [Qemu-devel] [Spice-devel] [Xen-devel] Qemu 2.0 regression with xen: qemu crash on any domUs S.O. start
On Mon, Apr 07, 2014 at 11:59:06AM +0200, Fabio Fantoni wrote: Today I did some tests also with hvm and spice and I found another segfault with different backtrace to solve: (gdb) c Continuing. *Program received signal SIGSEGV, Segmentation fault.** **0x55855d30 in interface_client_monitors_config (sin=0x563b0260, ** **mc=0x0) at ui/spice-display.c:557** **557 if (mc-num_of_monitors 0) {* (gdb) bt full #0 0x55855d30 in interface_client_monitors_config ( sin=0x563b0260, mc=0x0) at ui/spice-display.c:557 ssd = 0x563b0210 info = {xoff = 0, yoff = 0, width = 0, height = 0} rc = 32767 __func__ = interface_client_monitors_config #1 0x74af5113 in ?? () from /usr/lib/x86_64-linux-gnu/libspice-server.so.1 No symbol table info available. A backtrace with spice-server debugging symbols installed would be helpful. Christophe pgpLb9qHAjCEt.pgp Description: PGP signature
[Qemu-devel] [PULL 1/1] spice: monitors_config: check pointer before dereferencing
Reported-by: Fabio Fantoni fabio.fant...@m2r.biz Signed-off-by: Gerd Hoffmann kra...@redhat.com --- ui/spice-display.c | 4 1 file changed, 4 insertions(+) diff --git a/ui/spice-display.c b/ui/spice-display.c index e28698c..ce6b220 100644 --- a/ui/spice-display.c +++ b/ui/spice-display.c @@ -549,6 +549,10 @@ static int interface_client_monitors_config(QXLInstance *sin, QemuUIInfo info; int rc; +if (!mc) { +return 1; +} + /* * FIXME: multihead is tricky due to the way * spice has multihead implemented. -- 1.8.3.1
[Qemu-devel] [PULL for-2.0 0/1] spice: monitors_config: check pointer before dereferencing
Hi, Simple spice bugfix for 2.0. please pull, Gerd The following changes since commit 466e6e9d13d56bbb6da1d2396d7d6347df483af0: target-i386: reorder fields in cpu/msr_hyperv_hypercall subsection (2014-04-05 10:49:05 +0100) are available in the git repository at: git://anongit.freedesktop.org/spice/qemu tags/pull-spice-6 for you to fetch changes up to dc491cfc14074064ed54a872b62cce6ca1330644: spice: monitors_config: check pointer before dereferencing (2014-04-07 12:18:43 +0200) spice: monitors_config: check pointer before dereferencing Gerd Hoffmann (1): spice: monitors_config: check pointer before dereferencing ui/spice-display.c | 4 1 file changed, 4 insertions(+)
Re: [Qemu-devel] [PATCH 28/35] pc: propagate memory hotplug event to ACPI device
On Fri, Apr 04, 2014 at 03:36:53PM +0200, Igor Mammedov wrote: Notify PIIX4_PM/ICH9LPC device about hotplug event, so that it would send SCI to guest notifying about newly added memory. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/pc.c | 13 + 1 file changed, 13 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 734c6ee..ee5cf88 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -60,6 +60,8 @@ #include acpi-build.h #include hw/mem/dimm.h #include trace.h +#include hw/acpi/piix4.h +#include hw/i386/ich9.h /* debug PC/ISA interrupts */ //#define DEBUG_IRQ @@ -1484,6 +1486,8 @@ void qemu_register_pc_machine(QEMUMachine *m) static void pc_dimm_plug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { +Object *acpi_dev; +HotplugHandlerClass *hhc; Error *local_err = NULL; PCMachineState *pcms = PC_MACHINE(hotplug_dev); DimmDevice *dimm = DIMM(dev); @@ -1517,10 +1521,19 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev, } trace_mhp_pc_dimm_assigned_slot(dimm-slot); +acpi_dev = (acpi_dev = piix4_pm_find()) ? acpi_dev : ich9_lpc_find(); +if (!acpi_dev) { +error_setg(local_err, + memory hotplug is not enabled: missing acpi device); +return; +} + +hhc = HOTPLUG_HANDLER_GET_CLASS(acpi_dev); How about simply looking for a hotplug handler type device instead? We aren't likely to have many of these, are we? memory_region_add_subregion(pcms-hotplug_memory, addr - pcms-hotplug_memory_base, mr); vmstate_register_ram(mr, dev); +hhc-plug(HOTPLUG_HANDLER(acpi_dev), dev, local_err); out: error_propagate(errp, local_err); -- 1.9.0
[Qemu-devel] [PATCH] pass an inclusive address range to xc_domain_pin_memory_cacheattr
xc_domain_pin_memory_cacheattr expects an inclusive address range: adjust the parameters. Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com diff --git a/xen-all.c b/xen-all.c index ba34739..027e7a8 100644 --- a/xen-all.c +++ b/xen-all.c @@ -323,7 +323,7 @@ go_physmap: xc_domain_pin_memory_cacheattr(xen_xc, xen_domid, start_addr TARGET_PAGE_BITS, - (start_addr + size) TARGET_PAGE_BITS, + (start_addr + size - 1) TARGET_PAGE_BITS, XEN_DOMCTL_MEM_CACHEATTR_WB); snprintf(path, sizeof(path),
[Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
If a pci-2-pci bridge supports hot-plug functionality but there are no devices connected to it, reserve IO/mem in order to be able to attach devices later. Do not waste space, use minimum allowed. Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- src/fw/pciinit.c | 3 +++ src/hw/pci.c | 17 + src/hw/pci.h | 1 + 3 files changed, 21 insertions(+) diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c index 64f1d41..9b5d7ad 100644 --- a/src/fw/pciinit.c +++ b/src/fw/pciinit.c @@ -677,12 +677,15 @@ static int pci_bios_check_devices(struct pci_bus *busses) continue; struct pci_bus *parent = busses[pci_bdf_to_bus(s-bus_dev-bdf)]; int type; +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); for (type = 0; type PCI_REGION_TYPE_COUNT; type++) { u64 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; if (pci_region_align(s-r[type]) align) align = pci_region_align(s-r[type]); u64 sum = pci_region_sum(s-r[type]); +if (!sum shpc_cap) +sum = align; /* reserve min size for hot-plug */ u64 size = ALIGN(sum, align); int is64 = pci_bios_bridge_region_is64(s-r[type], s-bus_dev, type); diff --git a/src/hw/pci.c b/src/hw/pci.c index caf9265..ee8a7f1 100644 --- a/src/hw/pci.c +++ b/src/hw/pci.c @@ -225,6 +225,23 @@ pci_find_init_device(const struct pci_device_id *ids, void *arg) return NULL; } +u8 pci_find_capability(struct pci_device *pci, u8 cap_id) +{ +u8 cap; +u16 status = pci_config_readw(pci-bdf, PCI_STATUS); + +if (!(status PCI_STATUS_CAP_LIST)) +return 0; + +for (cap = pci_config_readb(pci-bdf, PCI_CAPABILITY_LIST); cap; +cap = pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_NEXT)) +if (pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_ID) == cap_id) +return cap; + +return 0; +} + + void pci_reboot(void) { diff --git a/src/hw/pci.h b/src/hw/pci.h index 167a027..e828225 100644 --- a/src/hw/pci.h +++ b/src/hw/pci.h @@ -116,6 +116,7 @@ int pci_init_device(const struct pci_device_id *ids , struct pci_device *pci, void *arg); struct pci_device *pci_find_init_device(const struct pci_device_id *ids , void *arg); +u8 pci_find_capability(struct pci_device *pci, u8 cap_id); void pci_reboot(void); #endif -- 1.8.3.1
Re: [Qemu-devel] [PATCH 02/97] savevm: Remove all the unneded version_minimum_id_old (Massive)
On 7 April 2014 04:20, Juan Quintela quint...@redhat.com wrote: After previous Peter patch, they are redundant. This way we don't asign them except when needed. Once there, there were lots of case where the .fields indentation was wrong: .fields = (VMStateField []) { and .fields = (VMStateField []) { Change all the combinations to: .fields = (VMStateField[]){ The biggest problem (appart of aesthetics) was that checkpatch complained when we copypasted the code from one place to another. 211 files changed, 289 insertions(+), 621 deletions(-) I'm really not a fan of this kind of single patch that touches a huge number of files at once. They're basically impossible to review and they introduce the possibility of conflicts between submaintainer tree changes and the big patch. There's no reason to have all these changes in a single patch -- I'd much rather see one patch per subsystem sent to the relevant submaintainers, plus one for all the unmaintained stuff which can go via the migration tree. thanks -- PMM
Re: [Qemu-devel] [PATCH v3 11/26] tcg-aarch64: Reuse LR in translated code
On 07.04.2014 11:49, Peter Maydell wrote: On 7 April 2014 09:03, Claudio Fontana claudio.font...@huawei.com wrote: On 03.04.2014 21:56, Richard Henderson wrote: It's obviously call-clobbered, but is otherwise unused. Repurpose it as the TCG temporary. Giving one last chance to the ARM guys to speak up about repurposing LR. Can you clarify what you think the issue is with using LR? I think you said before but I forget :-( My doubt was about the AAPCS64 (Procedure Call standard for the ARM 64-bit Architecture), and what the platforms in our case dictate regarding FP and LR use. I think that LR should be ok to use, because basically the whole generated code from the prologue to the end can be seen as a single big subroutine, and there does not seem to be a clear mandate to keep LR's special significance inside subroutines at all times. The role of the registers is described in 5.1.1, where it is mentioned that in all variants of the pcs, registers r16,r17,r29 and r30 have special roles [...] The standard says at 5.2.3 that conforming code shall construct a linked list of stack frames, [...] A platform shall mandate the minimum level of conformance[...]. Options are: * It may require the frame pointer to address a valid frame record at all times, except that small subroutines which do not modify the link register may elect not to create a frame record * It may require the frame pointer to address a valid frame record at all times, except that any subroutine may elect not to create a frame record * It may permit the frame pointer register to be used as a general-purpose callee-saved register, but provide a platform-specific mechanism for external agents to reliably detect this condition * It may elect not to maintain a frame chain and to use the frame pointer register as a general-purpose callee-saved register. I think however that since the latest version of RH's patches do not repurpose FP, this is not that relevant anymore, but I think that the general question remains topical, ie, which of these options do our platforms dictate? Thanks, Claudio
Re: [Qemu-devel] [PATCH v3 11/26] tcg-aarch64: Reuse LR in translated code
On 7 April 2014 12:11, Claudio Fontana claudio.font...@huawei.com wrote: [your mail client is generating very long lines] My doubt was about the AAPCS64 (Procedure Call standard for the ARM 64-bit Architecture), and what the platforms in our case dictate regarding FP and LR use. I think that LR should be ok to use, because basically the whole generated code from the prologue to the end can be seen as a single big subroutine, and there does not seem to be a clear mandate to keep LR's special significance inside subroutines at all times. Agreed. The role of the registers is described in 5.1.1, where it is mentioned that in all variants of the pcs, registers r16,r17,r29 and r30 have special roles [...] The standard says at 5.2.3 that conforming code shall construct a linked list of stack frames, [...] A platform shall mandate the minimum level of conformance[...]. Since our TCG generated code is not platform specific we should take the most conservative approach, and make sure we set up a frame record in the prologue and don't touch FP after that. thanks -- PMM
Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic
On Fri, Apr 04, 2014 at 03:36:48PM +0200, Igor Mammedov wrote: ... and report error if plugged in device is not supported. Later generic callbacks will be used by memory hotplug. Signed-off-by: Igor Mammedov imamm...@redhat.com OK in that case, how about teaching all hotplug callbacks about this? There are two ATM: shpc_device_hotplug_cb pcie_cap_slot_hotplug_cb Teach them both to fail gracefully if they get an object that is not a pci device. Afterwards, simply iterate over all objects of type TYPE_HOTPLUG_HANDLER and look for one that will accept your object. --- hw/acpi/piix4.c | 31 ++- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 67dc075..4341f82 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -310,19 +310,32 @@ static void piix4_pm_powerdown_req(Notifier *n, void *opaque) acpi_pm1_evt_power_down(s-ar); } -static void piix4_pci_device_plug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_plug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); -acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, errp); + +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { +acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, + errp); +} else { +error_setg(errp, acpi: device plug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} } -static void piix4_pci_device_unplug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); -acpi_pcihp_device_unplug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, -errp); + +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { +acpi_pcihp_device_unplug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, +errp); +} else { +error_setg(errp, acpi: device unplug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} } static void piix4_update_bus_hotplug(PCIBus *pci_bus, void *opaque) @@ -553,8 +566,8 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data) */ dc-cannot_instantiate_with_device_add_yet = true; dc-hotpluggable = false; -hc-plug = piix4_pci_device_plug_cb; -hc-unplug = piix4_pci_device_unplug_cb; +hc-plug = piix4_device_plug_cb; +hc-unplug = piix4_device_unplug_cb; } static const TypeInfo piix4_pm_info = { -- 1.9.0
[Qemu-devel] LAST CALL for 2.0 patches/pulls
Hi; the release schedule for 2.0 is currently that we will tag an rc2 on Tuesday, and (assuming no critical issues found in testing it) that will then be tagged as the final 2.0 on Wednesday. So if you have any further pull requests for 2.0 please get them onto the list by the end of today (Monday) so I can apply them either today or tomorrow morning; I plan to tag rc2 lunchtimeish UK time Tuesday. If there are patches which should go into 2.0 which haven't yet been picked up please reply to this email with a pointer to them. thanks -- PMM
Re: [Qemu-devel] [PATCH v3 24/26] tcg-aarch64: Replace aarch64_ldst_op_data with AArch64LdstType
On 03.04.2014 21:56, Richard Henderson wrote: The definition of op_type wasn't encoded for the proper shift for the field, making the implementations confusing. Signed-off-by: Richard Henderson r...@twiddle.net At the end of the day the magic values remain in the load/store instructions though. Can we find a way to replace them with INSN_-something like for the others? I think I was doing something of the sort in a now obsolete patch I suggested some time early this year, see if it helps: http://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05074.html Claudio --- tcg/aarch64/tcg-target.c | 42 +- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c index 9a2e4a6..a538a87 100644 --- a/tcg/aarch64/tcg-target.c +++ b/tcg/aarch64/tcg-target.c @@ -242,12 +242,12 @@ static const enum aarch64_cond_code tcg_cond_to_aarch64[] = { [TCG_COND_LEU] = COND_LS, }; -enum aarch64_ldst_op_type { /* type of operation */ -LDST_ST = 0x0,/* store */ -LDST_LD = 0x4,/* load */ -LDST_LD_S_X = 0x8, /* load and sign-extend into Xt */ -LDST_LD_S_W = 0xc, /* load and sign-extend into Wt */ -}; +typedef enum { +LDST_ST = 0,/* store */ +LDST_LD = 1,/* load */ +LDST_LD_S_X = 2, /* load and sign-extend into Xt */ +LDST_LD_S_W = 3, /* load and sign-extend into Wt */ +} AArch64LdstType; /* We encode the format of the insn into the beginning of the name, so that we can have the preprocessor help typecheck the insn vs the output @@ -483,22 +483,19 @@ static void tcg_out_insn_3509(TCGContext *s, AArch64Insn insn, TCGType ext, } -static inline void tcg_out_ldst_9(TCGContext *s, TCGMemOp size, - enum aarch64_ldst_op_type op_type, - TCGReg rd, TCGReg rn, intptr_t offset) +static void tcg_out_ldst_9(TCGContext *s, TCGMemOp size, AArch64LdstType type, + TCGReg rd, TCGReg rn, intptr_t offset) { /* use LDUR with BASE register with 9bit signed unscaled offset */ -tcg_out32(s, 0x3800 | size 30 | op_type 20 +tcg_out32(s, 0x3800 | size 30 | type 22 | (offset 0x1ff) 12 | rn 5 | rd); } /* tcg_out_ldst_12 expects a scaled unsigned immediate offset */ -static inline void tcg_out_ldst_12(TCGContext *s, TCGMemOp size, - enum aarch64_ldst_op_type op_type, - TCGReg rd, TCGReg rn, - tcg_target_ulong scaled_uimm) +static void tcg_out_ldst_12(TCGContext *s, TCGMemOp size, AArch64LdstType type, +TCGReg rd, TCGReg rn, tcg_target_ulong scaled_uimm) { -tcg_out32(s, 0x3900 | size 30 | op_type 20 +tcg_out32(s, 0x3900 | size 30 | type 22 | scaled_uimm 10 | rn 5 | rd); } @@ -637,21 +634,16 @@ static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg rd, } } -static inline void tcg_out_ldst_r(TCGContext *s, TCGMemOp size, - enum aarch64_ldst_op_type op_type, - TCGReg rd, TCGReg base, TCGReg regoff) +static void tcg_out_ldst_r(TCGContext *s, TCGMemOp size, AArch64LdstType type, + TCGReg rd, TCGReg base, TCGReg regoff) { -/* load from memory to register using base + 64bit register offset */ -/* using f.e. STR Wt, [Xn, Xm] 0xb8600800|(regoff 16)|(base 5)|rd */ -/* the 0x6000 is for the no extend field */ -tcg_out32(s, 0x38206800 | size 30 | op_type 20 +tcg_out32(s, 0x38206800 | size 30 | type 22 | regoff 16 | base 5 | rd); } /* solve the whole ldst problem */ -static inline void tcg_out_ldst(TCGContext *s, TCGMemOp size, -enum aarch64_ldst_op_type type, -TCGReg rd, TCGReg rn, intptr_t offset) +static void tcg_out_ldst(TCGContext *s, TCGMemOp size, AArch64LdstType type, + TCGReg rd, TCGReg rn, intptr_t offset) { if (offset = -256 offset 256) { tcg_out_ldst_9(s, size, type, rd, rn, offset);
Re: [Qemu-devel] [PATCH 02/97] savevm: Remove all the unneded version_minimum_id_old (Massive)
Peter Maydell peter.mayd...@linaro.org wrote: On 7 April 2014 04:20, Juan Quintela quint...@redhat.com wrote: After previous Peter patch, they are redundant. This way we don't asign them except when needed. Once there, there were lots of case where the .fields indentation was wrong: .fields = (VMStateField []) { and .fields = (VMStateField []) { Change all the combinations to: .fields = (VMStateField[]){ The biggest problem (appart of aesthetics) was that checkpatch complained when we copypasted the code from one place to another. 211 files changed, 289 insertions(+), 621 deletions(-) I'm really not a fan of this kind of single patch that touches a huge number of files at once. They're basically impossible to review and they introduce the possibility of conflicts between submaintainer tree changes and the big patch. There's no reason to have all these changes in a single patch -- I'd much rather see one patch per subsystem sent to the relevant submaintainers, plus one for all the unmaintained stuff which can go via the migration tree. If you say how to split, I am all for it. But remomeber that all this changes go inside VMSTateDescription descriptions, touch nothing else outside of that. And that is not something that is touched a lot to have lots of conflicts. My problem with leaving things as are, is that people continue to copy from the ones that still use the old system/format :-( Later, Juan.
Re: [Qemu-devel] [PULL for-2.0 0/4] gtk: pointer fixes
On 7 April 2014 09:56, Gerd Hoffmann kra...@redhat.com wrote: Hi, Last minute pointer fixes for gtk. And a MAINTAINERS update to help ui patches being picked up more timely in the future. please pull, Gerd The following changes since commit 466e6e9d13d56bbb6da1d2396d7d6347df483af0: target-i386: reorder fields in cpu/msr_hyperv_hypercall subsection (2014-04-05 10:49:05 +0100) are available in the git repository at: git://git.kraxel.org/qemu tags/pull-gtk-4 for you to fetch changes up to 25eccc37ff91254efdd123f5dafb37526a83a990: ui: Update MAINTAINERS entry. (2014-04-07 10:50:30 +0200) Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 02/97] savevm: Remove all the unneded version_minimum_id_old (Massive)
On 7 April 2014 12:47, Juan Quintela quint...@redhat.com wrote: Peter Maydell peter.mayd...@linaro.org wrote: I'm really not a fan of this kind of single patch that touches a huge number of files at once. They're basically impossible to review and they introduce the possibility of conflicts between submaintainer tree changes and the big patch. There's no reason to have all these changes in a single patch -- I'd much rather see one patch per subsystem sent to the relevant submaintainers, plus one for all the unmaintained stuff which can go via the migration tree. If you say how to split, I am all for it. Like I said, one patch per maintained subsystem, one patch for the leftovers. thanks -- PMM
Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Mo, 2014-04-07 at 13:59 +0300, Marcel Apfelbaum wrote: If a pci-2-pci bridge supports hot-plug functionality but there are no devices connected to it, reserve IO/mem in order to be able to attach devices later. Do not waste space, use minimum allowed. Makes sense. +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); Should we also check for hotplug-capable pci express ports while being at it? cheers, Gerd
Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic
On Mon, 7 Apr 2014 14:32:41 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 04, 2014 at 03:36:48PM +0200, Igor Mammedov wrote: ... and report error if plugged in device is not supported. Later generic callbacks will be used by memory hotplug. Signed-off-by: Igor Mammedov imamm...@redhat.com OK in that case, how about teaching all hotplug callbacks about this? There are two ATM: shpc_device_hotplug_cb pcie_cap_slot_hotplug_cb Teach them both to fail gracefully if they get an object that is not a pci device. Afterwards, simply iterate over all objects of type TYPE_HOTPLUG_HANDLER and look for one that will accept your object. Then you would never know if any hotplug handler has actually handled event. I think hotplug handler should return error if unsupported device passed in rather than ignore it. It makes catching wiring errors easier. Dropping error so that we could not care which hotplug handler should be notified, looks like a wrong direction and makes system more fragile. It shouldn't be up to consumer to determine that event should be routed to it, but rather by external routing that knows what and when should be notified. --- hw/acpi/piix4.c | 31 ++- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 67dc075..4341f82 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -310,19 +310,32 @@ static void piix4_pm_powerdown_req(Notifier *n, void *opaque) acpi_pm1_evt_power_down(s-ar); } -static void piix4_pci_device_plug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_plug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); -acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, errp); + +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { +acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, + errp); +} else { +error_setg(errp, acpi: device plug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} } -static void piix4_pci_device_unplug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); -acpi_pcihp_device_unplug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, -errp); + +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { +acpi_pcihp_device_unplug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, +errp); +} else { +error_setg(errp, acpi: device unplug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} } static void piix4_update_bus_hotplug(PCIBus *pci_bus, void *opaque) @@ -553,8 +566,8 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data) */ dc-cannot_instantiate_with_device_add_yet = true; dc-hotpluggable = false; -hc-plug = piix4_pci_device_plug_cb; -hc-unplug = piix4_pci_device_unplug_cb; +hc-plug = piix4_device_plug_cb; +hc-unplug = piix4_device_unplug_cb; } static const TypeInfo piix4_pm_info = { -- 1.9.0
Re: [Qemu-devel] [PULL for-2.0 0/1] spice: monitors_config: check pointer before dereferencing
On 7 April 2014 11:21, Gerd Hoffmann kra...@redhat.com wrote: Hi, Simple spice bugfix for 2.0. please pull, Gerd The following changes since commit 466e6e9d13d56bbb6da1d2396d7d6347df483af0: target-i386: reorder fields in cpu/msr_hyperv_hypercall subsection (2014-04-05 10:49:05 +0100) are available in the git repository at: git://anongit.freedesktop.org/spice/qemu tags/pull-spice-6 for you to fetch changes up to dc491cfc14074064ed54a872b62cce6ca1330644: spice: monitors_config: check pointer before dereferencing (2014-04-07 12:18:43 +0200) spice: monitors_config: check pointer before dereferencing Gerd Hoffmann (1): spice: monitors_config: check pointer before dereferencing ui/spice-display.c | 4 1 file changed, 4 insertions(+) Applied, thanks. -- PMM
Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic
On Mon, Apr 07, 2014 at 02:00:37PM +0200, Igor Mammedov wrote: On Mon, 7 Apr 2014 14:32:41 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 04, 2014 at 03:36:48PM +0200, Igor Mammedov wrote: ... and report error if plugged in device is not supported. Later generic callbacks will be used by memory hotplug. Signed-off-by: Igor Mammedov imamm...@redhat.com OK in that case, how about teaching all hotplug callbacks about this? There are two ATM: shpc_device_hotplug_cb pcie_cap_slot_hotplug_cb Teach them both to fail gracefully if they get an object that is not a pci device. Afterwards, simply iterate over all objects of type TYPE_HOTPLUG_HANDLER and look for one that will accept your object. Then you would never know if any hotplug handler has actually handled event. Why not? Check the error. If no one accepts your object, return error to user. I think hotplug handler should return error if unsupported device passed in rather than ignore it. It makes catching wiring errors easier. Absolutely. Dropping error so that we could not care which hotplug handler should be notified, looks like a wrong direction and makes system more fragile. That's not what I was suggesting. It shouldn't be up to consumer to determine that event should be routed to it, but rather by external routing that knows what and when should be notified. Yes. So for each hotplug handler (err) handler-plug(device, err) if (!err) break; if (err) hotplug failed - destroy device --- hw/acpi/piix4.c | 31 ++- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 67dc075..4341f82 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -310,19 +310,32 @@ static void piix4_pm_powerdown_req(Notifier *n, void *opaque) acpi_pm1_evt_power_down(s-ar); } -static void piix4_pci_device_plug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_plug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); -acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, errp); + +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { +acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, + errp); +} else { +error_setg(errp, acpi: device plug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} } -static void piix4_pci_device_unplug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); -acpi_pcihp_device_unplug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, -errp); + +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { +acpi_pcihp_device_unplug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, +errp); +} else { +error_setg(errp, acpi: device unplug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} } static void piix4_update_bus_hotplug(PCIBus *pci_bus, void *opaque) @@ -553,8 +566,8 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data) */ dc-cannot_instantiate_with_device_add_yet = true; dc-hotpluggable = false; -hc-plug = piix4_pci_device_plug_cb; -hc-unplug = piix4_pci_device_unplug_cb; +hc-plug = piix4_device_plug_cb; +hc-unplug = piix4_device_unplug_cb; } static const TypeInfo piix4_pm_info = { -- 1.9.0
Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Mon, Apr 07, 2014 at 02:01:41PM +0200, Gerd Hoffmann wrote: On Mo, 2014-04-07 at 13:59 +0300, Marcel Apfelbaum wrote: If a pci-2-pci bridge supports hot-plug functionality but there are no devices connected to it, reserve IO/mem in order to be able to attach devices later. Do not waste space, use minimum allowed. Makes sense. +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); Should we also check for hotplug-capable pci express ports while being at it? cheers, Gerd Can be a separate patch?
Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Mon, Apr 07, 2014 at 01:59:02PM +0300, Marcel Apfelbaum wrote: If a pci-2-pci bridge supports hot-plug functionality but there are no devices connected to it, reserve IO/mem in order to be able to attach devices later. Do not waste space, use minimum allowed. Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- src/fw/pciinit.c | 3 +++ src/hw/pci.c | 17 + src/hw/pci.h | 1 + 3 files changed, 21 insertions(+) diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c index 64f1d41..9b5d7ad 100644 --- a/src/fw/pciinit.c +++ b/src/fw/pciinit.c @@ -677,12 +677,15 @@ static int pci_bios_check_devices(struct pci_bus *busses) continue; struct pci_bus *parent = busses[pci_bdf_to_bus(s-bus_dev-bdf)]; int type; +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); for (type = 0; type PCI_REGION_TYPE_COUNT; type++) { u64 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; if (pci_region_align(s-r[type]) align) align = pci_region_align(s-r[type]); u64 sum = pci_region_sum(s-r[type]); +if (!sum shpc_cap) +sum = align; /* reserve min size for hot-plug */ u64 size = ALIGN(sum, align); int is64 = pci_bios_bridge_region_is64(s-r[type], s-bus_dev, type); One thing I'd do is maybe check that the relevant memory type is enabled in the bridge (probably just by writing fff to base and reading it back). This will give hypervisors an option to avoid wasting resources: e.g. it's uncommon for express devices to claim IO. diff --git a/src/hw/pci.c b/src/hw/pci.c index caf9265..ee8a7f1 100644 --- a/src/hw/pci.c +++ b/src/hw/pci.c @@ -225,6 +225,23 @@ pci_find_init_device(const struct pci_device_id *ids, void *arg) return NULL; } +u8 pci_find_capability(struct pci_device *pci, u8 cap_id) +{ +u8 cap; +u16 status = pci_config_readw(pci-bdf, PCI_STATUS); + +if (!(status PCI_STATUS_CAP_LIST)) +return 0; + +for (cap = pci_config_readb(pci-bdf, PCI_CAPABILITY_LIST); cap; +cap = pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_NEXT)) +if (pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_ID) == cap_id) +return cap; I would also limit this to 256 iterations, to make sure we dont' get into an infinite loop with a broken device. + +return 0; +} + + void pci_reboot(void) { diff --git a/src/hw/pci.h b/src/hw/pci.h index 167a027..e828225 100644 --- a/src/hw/pci.h +++ b/src/hw/pci.h @@ -116,6 +116,7 @@ int pci_init_device(const struct pci_device_id *ids , struct pci_device *pci, void *arg); struct pci_device *pci_find_init_device(const struct pci_device_id *ids , void *arg); +u8 pci_find_capability(struct pci_device *pci, u8 cap_id); void pci_reboot(void); #endif -- 1.8.3.1
Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Mon, 2014-04-07 at 15:11 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 02:01:41PM +0200, Gerd Hoffmann wrote: On Mo, 2014-04-07 at 13:59 +0300, Marcel Apfelbaum wrote: If a pci-2-pci bridge supports hot-plug functionality but there are no devices connected to it, reserve IO/mem in order to be able to attach devices later. Do not waste space, use minimum allowed. Makes sense. +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); Should we also check for hotplug-capable pci express ports while being at it? cheers, Gerd Can be a separate patch? I think it can be. This one handles pci-2-pci bridges and avoids PCIe by choice. I do plan to handle PCIe hotplug corners later. Thanks, Marcel
Re: [Qemu-devel] [PATCH 02/97] savevm: Remove all the unneded version_minimum_id_old (Massive)
Peter Maydell peter.mayd...@linaro.org writes: On 7 April 2014 12:47, Juan Quintela quint...@redhat.com wrote: Peter Maydell peter.mayd...@linaro.org wrote: I'm really not a fan of this kind of single patch that touches a huge number of files at once. They're basically impossible to review and they introduce the possibility of conflicts between submaintainer tree changes and the big patch. There's no reason to have all these changes in a single patch -- I'd much rather see one patch per subsystem sent to the relevant submaintainers, plus one for all the unmaintained stuff which can go via the migration tree. If you say how to split, I am all for it. Like I said, one patch per maintained subsystem, one patch for the leftovers. Easier said than done. MAINTAINERS has more than 100 sections, yet it leaves more than 1200 files uncovered, roughly half of them C sources. I doubt splitting mechanically along those sections plus a catch-all patch for the unmaintained files would be appreciated. Could you give some guidance on splitting?
Re: [Qemu-devel] [PATCH 02/97] savevm: Remove all the unneded version_minimum_id_old (Massive)
On 7 April 2014 13:21, Markus Armbruster arm...@redhat.com wrote: Peter Maydell peter.mayd...@linaro.org writes: On 7 April 2014 12:47, Juan Quintela quint...@redhat.com wrote: Peter Maydell peter.mayd...@linaro.org wrote: Like I said, one patch per maintained subsystem, one patch for the leftovers. Easier said than done. MAINTAINERS has more than 100 sections, yet it leaves more than 1200 files uncovered, roughly half of them C sources. I doubt splitting mechanically along those sections plus a catch-all patch for the unmaintained files would be appreciated. Could you give some guidance on splitting? Well, I would just eyeball the filenames. If you had a patch with the ARM related stuff, a patch for the things that are x86 PC devices, one for the PPC devices, and a USB patch, that ought to whittle the list down a fair amount. thanks -- PMM
Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
Hi, +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); One thing I'd do is maybe check that the relevant memory type is enabled in the bridge (probably just by writing fff to base and reading it back). This will give hypervisors an option to avoid wasting resources: e.g. it's uncommon for express devices to claim IO. I don't think we'll need that for the SHPC bridge. For express it indeed makes sense to avoid claiming IO address space. I'd try to find something more automatic though, where you don't need some kind of disable io for this express port config option. For express ports which can only have a single device underneath we can check whenever we have a device and if one is present already don't bother claiming extra resources for hotplug. +for (cap = pci_config_readb(pci-bdf, PCI_CAPABILITY_LIST); cap; +cap = pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_NEXT)) +if (pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_ID) == cap_id) +return cap; I would also limit this to 256 iterations, to make sure we dont' get into an infinite loop with a broken device. Good point. cheers, Gerd
Re: [Qemu-devel] [PATCH 02/97] savevm: Remove all the unneded version_minimum_id_old (Massive)
Markus Armbruster arm...@redhat.com wrote: Peter Maydell peter.mayd...@linaro.org writes: On 7 April 2014 12:47, Juan Quintela quint...@redhat.com wrote: Peter Maydell peter.mayd...@linaro.org wrote: I'm really not a fan of this kind of single patch that touches a huge number of files at once. They're basically impossible to review and they introduce the possibility of conflicts between submaintainer tree changes and the big patch. There's no reason to have all these changes in a single patch -- I'd much rather see one patch per subsystem sent to the relevant submaintainers, plus one for all the unmaintained stuff which can go via the migration tree. If you say how to split, I am all for it. Like I said, one patch per maintained subsystem, one patch for the leftovers. Easier said than done. MAINTAINERS has more than 100 sections, yet it leaves more than 1200 files uncovered, roughly half of them C sources. I doubt splitting mechanically along those sections plus a catch-all patch for the unmaintained files would be appreciated. Could you give some guidance on splitting? +1
Re: [Qemu-devel] [PATCH 02/97] savevm: Remove all the unneded version_minimum_id_old (Massive)
Peter Maydell peter.mayd...@linaro.org wrote: On 7 April 2014 13:21, Markus Armbruster arm...@redhat.com wrote: Peter Maydell peter.mayd...@linaro.org writes: On 7 April 2014 12:47, Juan Quintela quint...@redhat.com wrote: Peter Maydell peter.mayd...@linaro.org wrote: Like I said, one patch per maintained subsystem, one patch for the leftovers. Easier said than done. MAINTAINERS has more than 100 sections, yet it leaves more than 1200 files uncovered, roughly half of them C sources. I doubt splitting mechanically along those sections plus a catch-all patch for the unmaintained files would be appreciated. Could you give some guidance on splitting? Well, I would just eyeball the filenames. If you had a patch with the ARM related stuff, a patch for the things that are x86 PC devices, one for the PPC devices, and a USB patch, that ought to whittle the list down a fair amount. Thanks. Will do. Later, Juan.
Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Mon, 2014-04-07 at 14:44 +0200, Gerd Hoffmann wrote: Hi, +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); One thing I'd do is maybe check that the relevant memory type is enabled in the bridge (probably just by writing fff to base and reading it back). This will give hypervisors an option to avoid wasting resources: e.g. it's uncommon for express devices to claim IO. I don't think we'll need that for the SHPC bridge. I agree. This is why I chose to check shpc capability, because PCIe ports do not use it (as far as I know). For express it indeed makes sense to avoid claiming IO address space. I'd try to find something more automatic though, where you don't need some kind of disable io for this express port config option. For express ports which can only have a single device underneath we can check whenever we have a device and if one is present already don't bother claiming extra resources for hotplug. +for (cap = pci_config_readb(pci-bdf, PCI_CAPABILITY_LIST); cap; +cap = pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_NEXT)) +if (pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_ID) == cap_id) +return cap; I would also limit this to 256 iterations, to make sure we dont' get into an infinite loop with a broken device. Good point. Yes, thanks! sending v2. Marcel cheers, Gerd
[Qemu-devel] [PATCH] spapr_pci: Fix number of returned vectors in ibm, change-msi
Current guest kernels try allocating as many vectors as the quota is. For example, in the case of virtio-net (which has just 3 vectors) the guest requests 4 vectors (that is the quota in the test) and the existing ibm,change-msi handler returns 4. But before it returns, it calls msix_set_message() in a loop and corrupts memory behind the end of msix_table. This limits the number of vectors returned by ibm,change-msi to the maximum supported by the actual device. Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru --- This might go to 2.0 actually. --- hw/ppc/spapr_pci.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index cbef095..cdfa369 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -343,6 +343,9 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPREnvironment *spapr, /* There is no cached config, allocate MSIs */ if (!phb-msi_table[ndev].nvec) { +if (req_num pdev-msix_entries_nr) { +req_num = pdev-msix_entries_nr; +} irq = spapr_allocate_irq_block(req_num, false, ret_intr_type == RTAS_TYPE_MSI); if (irq 0) { -- 1.8.4.rc4
[Qemu-devel] [SeaBIOS] [PATCH V2] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
If a pci-2-pci bridge supports hot-plug functionality but there are no devices connected to it, reserve IO/mem in order to be able to attach devices later. Do not waste space, use minimum allowed. Signed-off-by: Marcel Apfelbaum marce...@redhat.com --- - Thanks Gerd Hoffmann for the review. v1 - v2: - Addressed Michael S. Tsirkin's comments: - Limit capabilities query to 256 iterations, to make sure we don't get into an infinite loop with a broken device. src/fw/pciinit.c | 3 +++ src/hw/pci.c | 19 +++ src/hw/pci.h | 1 + 3 files changed, 23 insertions(+) diff --git a/src/fw/pciinit.c b/src/fw/pciinit.c index 64f1d41..9b5d7ad 100644 --- a/src/fw/pciinit.c +++ b/src/fw/pciinit.c @@ -677,12 +677,15 @@ static int pci_bios_check_devices(struct pci_bus *busses) continue; struct pci_bus *parent = busses[pci_bdf_to_bus(s-bus_dev-bdf)]; int type; +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); for (type = 0; type PCI_REGION_TYPE_COUNT; type++) { u64 align = (type == PCI_REGION_TYPE_IO) ? PCI_BRIDGE_IO_MIN : PCI_BRIDGE_MEM_MIN; if (pci_region_align(s-r[type]) align) align = pci_region_align(s-r[type]); u64 sum = pci_region_sum(s-r[type]); +if (!sum shpc_cap) +sum = align; /* reserve min size for hot-plug */ u64 size = ALIGN(sum, align); int is64 = pci_bios_bridge_region_is64(s-r[type], s-bus_dev, type); diff --git a/src/hw/pci.c b/src/hw/pci.c index caf9265..055353d 100644 --- a/src/hw/pci.c +++ b/src/hw/pci.c @@ -225,6 +225,25 @@ pci_find_init_device(const struct pci_device_id *ids, void *arg) return NULL; } +u8 pci_find_capability(struct pci_device *pci, u8 cap_id) +{ +int i; +u8 cap; +u16 status = pci_config_readw(pci-bdf, PCI_STATUS); + +if (!(status PCI_STATUS_CAP_LIST)) +return 0; + +for (i = 0, cap = pci_config_readb(pci-bdf, PCI_CAPABILITY_LIST); + (i = 0xff) cap; + i++, cap = pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_NEXT)) +if (pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_ID) == cap_id) +return cap; + +return 0; +} + + void pci_reboot(void) { diff --git a/src/hw/pci.h b/src/hw/pci.h index 167a027..e828225 100644 --- a/src/hw/pci.h +++ b/src/hw/pci.h @@ -116,6 +116,7 @@ int pci_init_device(const struct pci_device_id *ids , struct pci_device *pci, void *arg); struct pci_device *pci_find_init_device(const struct pci_device_id *ids , void *arg); +u8 pci_find_capability(struct pci_device *pci, u8 cap_id); void pci_reboot(void); #endif -- 1.8.3.1
Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic
On Mon, 7 Apr 2014 15:07:15 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Apr 07, 2014 at 02:00:37PM +0200, Igor Mammedov wrote: On Mon, 7 Apr 2014 14:32:41 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 04, 2014 at 03:36:48PM +0200, Igor Mammedov wrote: ... and report error if plugged in device is not supported. Later generic callbacks will be used by memory hotplug. Signed-off-by: Igor Mammedov imamm...@redhat.com OK in that case, how about teaching all hotplug callbacks about this? There are two ATM: shpc_device_hotplug_cb pcie_cap_slot_hotplug_cb Teach them both to fail gracefully if they get an object that is not a pci device. Afterwards, simply iterate over all objects of type TYPE_HOTPLUG_HANDLER and look for one that will accept your object. Then you would never know if any hotplug handler has actually handled event. Why not? Check the error. If no one accepts your object, return error to user. I think hotplug handler should return error if unsupported device passed in rather than ignore it. It makes catching wiring errors easier. Absolutely. Dropping error so that we could not care which hotplug handler should be notified, looks like a wrong direction and makes system more fragile. That's not what I was suggesting. It shouldn't be up to consumer to determine that event should be routed to it, but rather by external routing that knows what and when should be notified. Yes. So for each hotplug handler (err) handler-plug(device, err) if (!err) break; here it would break on the first handler that doesn't support device and tells so to caller. And there isn't any routing here, it just blindly broadcast to every handler, regardless whether it's right or not. If broadcast should be ever done than it probably should be a part of DEVICE class and part of [Qemu-devel] [PATCH 08/35] qdev: hotplug for buss-less devices patch and be generic to all devices. Andreas, since you care about QDEV do you have an opinion on ^^^ discussion? if (err) hotplug failed - destroy device --- hw/acpi/piix4.c | 31 ++- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 67dc075..4341f82 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -310,19 +310,32 @@ static void piix4_pm_powerdown_req(Notifier *n, void *opaque) acpi_pm1_evt_power_down(s-ar); } -static void piix4_pci_device_plug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_plug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); -acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, errp); + +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { +acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, + errp); +} else { +error_setg(errp, acpi: device plug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} } -static void piix4_pci_device_unplug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); -acpi_pcihp_device_unplug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, -errp); + +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { +acpi_pcihp_device_unplug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, +errp); +} else { +error_setg(errp, acpi: device unplug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} } static void piix4_update_bus_hotplug(PCIBus *pci_bus, void *opaque) @@ -553,8 +566,8 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data) */ dc-cannot_instantiate_with_device_add_yet = true; dc-hotpluggable = false; -hc-plug = piix4_pci_device_plug_cb; -hc-unplug = piix4_pci_device_unplug_cb; +hc-plug = piix4_device_plug_cb; +hc-unplug = piix4_device_unplug_cb; } static const TypeInfo piix4_pm_info = { -- 1.9.0
Re: [Qemu-devel] [PATCH for 2.1 00/97] VMState simplification (massive)
On Mon, 2014-04-07 at 05:20 +0200, Juan Quintela wrote: Hi Look at the diffstat. Almost all the additions are at test-vmstate.c. That is the reason why it is called a simplification. What this series does: - peter removal of version_minimum_id_old field when not needed (Peter) - cleanup: based on the previous one, I removed all the unneeded the uses on the tree. This should make your compiles a couple of nanoseconds faster. How did you do it :) ? I tried it with coccinelle but they have a problem handling compound literals :(. I opened a discussion on their mailing list hoping I will find a solution... https://systeme.lip6.fr/pipermail/cocci/2014-April/000794.html Thanks, Marcel - once there, fixed the indentation of the .fields line, to a canonical .fields = (VMStateField[]) [...]
Re: [Qemu-devel] [Spice-devel] [Xen-devel] Qemu 2.0 regression with xen: qemu crash on any domUs S.O. start
Il 07/04/2014 12:20, Christophe Fergeau ha scritto: On Mon, Apr 07, 2014 at 11:59:06AM +0200, Fabio Fantoni wrote: Today I did some tests also with hvm and spice and I found another segfault with different backtrace to solve: (gdb) c Continuing. *Program received signal SIGSEGV, Segmentation fault.** **0x55855d30 in interface_client_monitors_config (sin=0x563b0260, ** **mc=0x0) at ui/spice-display.c:557** **557 if (mc-num_of_monitors 0) {* (gdb) bt full #0 0x55855d30 in interface_client_monitors_config ( sin=0x563b0260, mc=0x0) at ui/spice-display.c:557 ssd = 0x563b0210 info = {xoff = 0, yoff = 0, width = 0, height = 0} rc = 32767 __func__ = interface_client_monitors_config #1 0x74af5113 in ?? () from /usr/lib/x86_64-linux-gnu/libspice-server.so.1 No symbol table info available. A backtrace with spice-server debugging symbols installed would be helpful. Christophe Sorry, the -dbg for spice-server on official debian packages is missing, now I created and installed also the -dbg package and this is the new backtrace: (gdb) c Continuing. Program received signal SIGSEGV, Segmentation fault. 0x55855d30 in interface_client_monitors_config (sin=0x563b0260, mc=0x0) at ui/spice-display.c:557 557 if (mc-num_of_monitors 0) { (gdb) bt full #0 0x55855d30 in interface_client_monitors_config ( sin=0x563b0260, mc=0x0) at ui/spice-display.c:557 ssd = 0x563b0210 info = {xoff = 0, yoff = 0, width = 0, height = 0} rc = 32767 __func__ = interface_client_monitors_config #1 0x74af5113 in red_dispatcher_use_client_monitors_config () at red_dispatcher.c:318 now = 0x563b0300 #2 0x74ad87f5 in agent_msg_filter_process_data ( filter=filter@entry=0x562eb0c4, data=data@entry=0x7fffe0280128 \001, len=328, len@entry=348) at agent-msg-filter.c:95 msg_header = {protocol = optimized out, type = optimized out, opaque = optimized out, size = 328, data = 0x831fd4 Address 0x831fd4 out of bounds} __FUNCTION__ = agent_msg_filter_process_data #3 0x74b1af76 in reds_on_main_agent_data (mcc=0x56326e70, message=0x7fffe0280128, size=348) at reds.c:1117 dev_state = 0x562eb0a8 header = optimized out res = optimized out __FUNCTION__ = reds_on_main_agent_data #4 0x74ae989a in main_channel_handle_parsed (rcc=0x56326e70, size=optimized out, type=optimized out, message=0x7fffe0280128) ---Type return to continue, or q return to quit--- at main_channel.c:911 main_chan = 0x562ef2b0 mcc = 0x56326e70 __FUNCTION__ = main_channel_handle_parsed #5 0x74aee470 in red_peer_handle_incoming (handler=0x5632af80, stream=0x565adba0) at red_channel.c:287 ret_handle = optimized out bytes_read = optimized out msg_type = 107 parsed = optimized out parsed_free = 0x74ba8620 nofree msg_size = 348 #6 red_channel_client_receive (rcc=rcc@entry=0x56326e70) at red_channel.c:309 No locals. #7 0x74af0d8c in red_channel_client_event (fd=optimized out, event=optimized out, data=0x56326e70) at red_channel.c:1435 rcc = 0x56326e70 #8 0x55851f82 in watch_read (opaque=0x5666e0a0) at ui/spice-core.c:101 watch = 0x5666e0a0 #9 0x557ce1f8 in qemu_iohandler_poll (pollfds=0x562e8e00, ret=1) at iohandler.c:143 revents = 1 pioh = 0x5634e080 ---Type return to continue, or q return to quit--- ioh = 0x5632fa30 #10 0x557cf2a4 in main_loop_wait (nonblocking=0) at main-loop.c:485 ret = 1 timeout = 4294967295 timeout_ns = 4237075 #11 0x5587acd8 in main_loop () at vl.c:2051 nonblocking = false last_io = 1 #12 0x558826b2 in main (argc=36, argv=0x7fffe358, envp=0x7fffe480) at vl.c:4507 i = 64 snapshot = 0 linux_boot = 0 icount_option = 0x0 initrd_filename = 0x0 kernel_filename = 0x0 kernel_cmdline = 0x55a1b5c4 boot_order = 0x562e7ee0 dc ds = 0x563d8fd0 cyls = 0 heads = 0 secs = 0 translation = 0 hda_opts = 0x0 opts = 0x562e7e30 ---Type return to continue, or q return to quit--- machine_opts = 0x562e84b0 olist = 0x55e00e00 optind = 36 optarg = 0x7fffe915 if=ide,index=1,media=cdrom,cache=writeback,id=ide-832 loadvm = 0x0 machine_class = 0x562e02a0 machine = 0x55e067e0 cpu_model = 0x0 vga_model = 0x0 qtest_chrdev = 0x0 qtest_log = 0x0 pid_file = 0x0 incoming = 0x0 show_vnc_port = 0 defconfig = true
Re: [Qemu-devel] [PATCH 28/35] pc: propagate memory hotplug event to ACPI device
On Mon, 7 Apr 2014 13:23:54 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 04, 2014 at 03:36:53PM +0200, Igor Mammedov wrote: Notify PIIX4_PM/ICH9LPC device about hotplug event, so that it would send SCI to guest notifying about newly added memory. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/pc.c | 13 + 1 file changed, 13 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 734c6ee..ee5cf88 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -60,6 +60,8 @@ #include acpi-build.h #include hw/mem/dimm.h #include trace.h +#include hw/acpi/piix4.h +#include hw/i386/ich9.h /* debug PC/ISA interrupts */ //#define DEBUG_IRQ @@ -1484,6 +1486,8 @@ void qemu_register_pc_machine(QEMUMachine *m) static void pc_dimm_plug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { +Object *acpi_dev; +HotplugHandlerClass *hhc; Error *local_err = NULL; PCMachineState *pcms = PC_MACHINE(hotplug_dev); DimmDevice *dimm = DIMM(dev); @@ -1517,10 +1521,19 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev, } trace_mhp_pc_dimm_assigned_slot(dimm-slot); +acpi_dev = (acpi_dev = piix4_pm_find()) ? acpi_dev : ich9_lpc_find(); +if (!acpi_dev) { +error_setg(local_err, + memory hotplug is not enabled: missing acpi device); +return; +} + +hhc = HOTPLUG_HANDLER_GET_CLASS(acpi_dev); How about simply looking for a hotplug handler type device instead? We aren't likely to have many of these, are we? There is at least 2 hotplug handlers that handle event for DIMM device, this one in PCMachine and in acpi device. Having explicit wiring where main handler forwards partially handled event to another known in advance handler would be more simple and robust approach. I think that's how real hardware works, i.e. when memory is hotplugged it doesn't triggers signals to CPU or SHCP hotplug circuits. Doing broadcast here would be overkill. memory_region_add_subregion(pcms-hotplug_memory, addr - pcms-hotplug_memory_base, mr); vmstate_register_ram(mr, dev); +hhc-plug(HOTPLUG_HANDLER(acpi_dev), dev, local_err); out: error_propagate(errp, local_err); -- 1.9.0
Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic
On Mon, Apr 07, 2014 at 03:12:11PM +0200, Igor Mammedov wrote: On Mon, 7 Apr 2014 15:07:15 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Apr 07, 2014 at 02:00:37PM +0200, Igor Mammedov wrote: On Mon, 7 Apr 2014 14:32:41 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 04, 2014 at 03:36:48PM +0200, Igor Mammedov wrote: ... and report error if plugged in device is not supported. Later generic callbacks will be used by memory hotplug. Signed-off-by: Igor Mammedov imamm...@redhat.com OK in that case, how about teaching all hotplug callbacks about this? There are two ATM: shpc_device_hotplug_cb pcie_cap_slot_hotplug_cb Teach them both to fail gracefully if they get an object that is not a pci device. Afterwards, simply iterate over all objects of type TYPE_HOTPLUG_HANDLER and look for one that will accept your object. Then you would never know if any hotplug handler has actually handled event. Why not? Check the error. If no one accepts your object, return error to user. I think hotplug handler should return error if unsupported device passed in rather than ignore it. It makes catching wiring errors easier. Absolutely. Dropping error so that we could not care which hotplug handler should be notified, looks like a wrong direction and makes system more fragile. That's not what I was suggesting. It shouldn't be up to consumer to determine that event should be routed to it, but rather by external routing that knows what and when should be notified. Yes. So for each hotplug handler (err) handler-plug(device, err) if (!err) break; here it would break on the first handler that doesn't support device and tells so to caller. This is pseudo-code, I really mean !err == no error reported. And there isn't any routing here, it just blindly broadcast to every handler, regardless whether it's right or not. Yes - handlers verify what they can support. If broadcast should be ever done than it probably should be a part of DEVICE class and part of [Qemu-devel] [PATCH 08/35] qdev: hotplug for buss-less devices patch and be generic to all devices. Not sure what's suggested here. Andreas, since you care about QDEV do you have an opinion on ^^^ discussion? if (err) hotplug failed - destroy device --- hw/acpi/piix4.c | 31 ++- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 67dc075..4341f82 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -310,19 +310,32 @@ static void piix4_pm_powerdown_req(Notifier *n, void *opaque) acpi_pm1_evt_power_down(s-ar); } -static void piix4_pci_device_plug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_plug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); -acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, errp); + +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { +acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, + errp); +} else { +error_setg(errp, acpi: device plug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} } -static void piix4_pci_device_unplug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); -acpi_pcihp_device_unplug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, -errp); + +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { +acpi_pcihp_device_unplug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, +errp); +} else { +error_setg(errp, acpi: device unplug request for not supported device +type: %s, object_get_typename(OBJECT(dev))); +} } static void piix4_update_bus_hotplug(PCIBus *pci_bus, void *opaque) @@ -553,8 +566,8 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data) */ dc-cannot_instantiate_with_device_add_yet = true; dc-hotpluggable = false; -hc-plug =
Re: [Qemu-devel] [PATCH for-2.0] Makefile: remove bashism
On 5 April 2014 15:25, Michael Tokarev m...@tls.msk.ru wrote: When installing modules (when --enable-modules is specified for ./configure), Makefile uses the following construct to replace all slashes with dashes in module name: ${s//\//-} This is a bash-specific substitution mechanism. POSIX does not have it, and some operating systems (for example Debian) does not implement this construct in default shell (for example dash). Use more traditional way to perform the substitution: use `tr' tool. Signed-off-By: Michael Tokarev m...@tls.msk.ru diff --git a/Makefile b/Makefile index ec74039..d622799 100644 --- a/Makefile +++ b/Makefile @@ -376,7 +376,7 @@ endif ifneq ($(CONFIG_MODULES),) $(INSTALL_DIR) $(DESTDIR)$(qemu_moddir) for s in $(patsubst %.mo,%$(DSOSUF),$(modules-m)); do \ - $(INSTALL_PROG) $(STRIP_OPT) $$s $(DESTDIR)$(qemu_moddir)/$${s//\//-}; \ + $(INSTALL_PROG) $(STRIP_OPT) $$s $(DESTDIR)$(qemu_moddir)/$$(echo $$s | tr / -); \ done endif ifneq ($(HELPERS-y),) Reviewed-by: Peter Maydell peter.mayd...@linaro.org Paolo, Fam: does this patch look ok to you? I propose to apply it for 2.0... thanks -- PMM
Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Mon, Apr 07, 2014 at 02:44:06PM +0200, Gerd Hoffmann wrote: Hi, +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); One thing I'd do is maybe check that the relevant memory type is enabled in the bridge (probably just by writing fff to base and reading it back). This will give hypervisors an option to avoid wasting resources: e.g. it's uncommon for express devices to claim IO. I don't think we'll need that for the SHPC bridge. Why not? I'm referring to this text in the bridge specification: The I/O Base and I/O Limit registers are optional and define an address range that is used by the bridge to determine when to forward I/O transactions from one interface to the other. If a bridge does not implement an I/O address range, then both the I/O Base and I/O Limit registers must be implemented as read-only registers that return zero when read. If a bridge supports an I/O address range, then these registers must be initialized by configuration software so default states are not specified. So we should probe bridge for I/O support before wasting I/O resources on it. The spec does not provide a way to detect this, but we can do it like this: - write value to I/O base register - read back value value 0 means bridge does not support I/O. A similar trick should work for other optional resources. For express it indeed makes sense to avoid claiming IO address space. I'd try to find something more automatic though, where you don't need some kind of disable io for this express port config option. Won't same trick as above work? For express ports which can only have a single device underneath we can check whenever we have a device and if one is present already don't bother claiming extra resources for hotplug. +for (cap = pci_config_readb(pci-bdf, PCI_CAPABILITY_LIST); cap; +cap = pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_NEXT)) +if (pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_ID) == cap_id) +return cap; I would also limit this to 256 iterations, to make sure we dont' get into an infinite loop with a broken device. Good point. cheers, Gerd
Re: [Qemu-devel] [PATCH for-2.0 v2] fw-path-provider: Change GPL version to 2+
Am 26.03.2014 15:19, schrieb Alexey Kardashevskiy: On 03/27/2014 01:13 AM, Alexey Kardashevskiy wrote: Cc: Andreas Färber afaer...@suse.de Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru Ah, this one is missing suggested-by. And Paolo's sob. Posting patches is tough :) --- Changes: v2: * corrected text Sorry, patch slipped through. I've transferred Paolo's Acked-by from v1 and tweaked the wording to match that at: https://www.gnu.org/licenses/old-licenses/gpl-2.0.en.html Thanks, applied to qom-next: https://github.com/afaerber/qemu-cpu/commits/qom-next Pull coming up shortly. 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] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Mon, 2014-04-07 at 16:34 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 02:44:06PM +0200, Gerd Hoffmann wrote: Hi, +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); One thing I'd do is maybe check that the relevant memory type is enabled in the bridge (probably just by writing fff to base and reading it back). This will give hypervisors an option to avoid wasting resources: e.g. it's uncommon for express devices to claim IO. I don't think we'll need that for the SHPC bridge. Why not? Because has shpc = not an PCIe port. (as far as I know) Anyway, why have shpc capability but no I/O or mem to support it? Thanks, Marcel I'm referring to this text in the bridge specification: The I/O Base and I/O Limit registers are optional and define an address range that is used by the bridge to determine when to forward I/O transactions from one interface to the other. If a bridge does not implement an I/O address range, then both the I/O Base and I/O Limit registers must be implemented as read-only registers that return zero when read. If a bridge supports an I/O address range, then these registers must be initialized by configuration software so default states are not specified. So we should probe bridge for I/O support before wasting I/O resources on it. The spec does not provide a way to detect this, but we can do it like this: - write value to I/O base register Why write? A simple read would be enough. It will never be 0(if I/O or mem is required) because of the Base Address part of the register which represents the address range, right? Thanks, Marcel - read back value value 0 means bridge does not support I/O. A similar trick should work for other optional resources. For express it indeed makes sense to avoid claiming IO address space. I'd try to find something more automatic though, where you don't need some kind of disable io for this express port config option. Won't same trick as above work? For express ports which can only have a single device underneath we can check whenever we have a device and if one is present already don't bother claiming extra resources for hotplug. +for (cap = pci_config_readb(pci-bdf, PCI_CAPABILITY_LIST); cap; +cap = pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_NEXT)) +if (pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_ID) == cap_id) +return cap; I would also limit this to 256 iterations, to make sure we dont' get into an infinite loop with a broken device. Good point. cheers, Gerd
Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Mon, 2014-04-07 at 16:51 +0300, Marcel Apfelbaum wrote: On Mon, 2014-04-07 at 16:34 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 02:44:06PM +0200, Gerd Hoffmann wrote: Hi, +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); One thing I'd do is maybe check that the relevant memory type is enabled in the bridge (probably just by writing fff to base and reading it back). This will give hypervisors an option to avoid wasting resources: e.g. it's uncommon for express devices to claim IO. I don't think we'll need that for the SHPC bridge. Why not? Because has shpc = not an PCIe port. (as far as I know) Anyway, why have shpc capability but no I/O or mem to support it? I signed too soon :), I have another question below, Thanks, Marcel [...] So we should probe bridge for I/O support before wasting I/O resources on it. The spec does not provide a way to detect this, but we can do it like this: - write value to I/O base register Why write? A simple read would be enough. It will never be 0(if I/O or mem is required) because of the Base Address part of the register which represents the address range, right? Here ^^^ Thanks, Marcel [...]
Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Mon, Apr 07, 2014 at 04:51:54PM +0300, Marcel Apfelbaum wrote: On Mon, 2014-04-07 at 16:34 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 02:44:06PM +0200, Gerd Hoffmann wrote: Hi, +u8 shpc_cap = pci_find_capability(s-bus_dev, PCI_CAP_ID_SHPC); One thing I'd do is maybe check that the relevant memory type is enabled in the bridge (probably just by writing fff to base and reading it back). This will give hypervisors an option to avoid wasting resources: e.g. it's uncommon for express devices to claim IO. I don't think we'll need that for the SHPC bridge. Why not? Because has shpc = not an PCIe port. (as far as I know) Anyway, why have shpc capability but no I/O or mem to support it? Thanks, Marcel I'm referring to this text in the bridge specification: The I/O Base and I/O Limit registers are optional and define an address range that is used by the bridge to determine when to forward I/O transactions from one interface to the other. If a bridge does not implement an I/O address range, then both the I/O Base and I/O Limit registers must be implemented as read-only registers that return zero when read. If a bridge supports an I/O address range, then these registers must be initialized by configuration software so default states are not specified. So we should probe bridge for I/O support before wasting I/O resources on it. The spec does not provide a way to detect this, but we can do it like this: - write value to I/O base register Why write? A simple read would be enough. It will never be 0(if I/O or mem is required) because of the Base Address part of the register which represents the address range, right? Thanks, Marcel AFAIK the spec does not list reset value for this register. IIRC QEMU resets both to 0. - read back value value 0 means bridge does not support I/O. A similar trick should work for other optional resources. For express it indeed makes sense to avoid claiming IO address space. I'd try to find something more automatic though, where you don't need some kind of disable io for this express port config option. Won't same trick as above work? For express ports which can only have a single device underneath we can check whenever we have a device and if one is present already don't bother claiming extra resources for hotplug. +for (cap = pci_config_readb(pci-bdf, PCI_CAPABILITY_LIST); cap; +cap = pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_NEXT)) +if (pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_ID) == cap_id) +return cap; I would also limit this to 256 iterations, to make sure we dont' get into an infinite loop with a broken device. Good point. cheers, Gerd
[Qemu-devel] [PATCH for-2.0 v2] tests: Don't run qom-test twice
Commit 3687d5325 accidentally resulted in running qom-test twice for x86_64, once directly via the wildcard, and once because x86_64 includes all the i386 qtests (which includes qom-test). Filter out x86_64 as well as microblazeel and xtensaeb to fix this. Cc: Peter Maydell peter.mayd...@linaro.org Signed-off-by: Andreas Färber afaer...@suse.de --- v1 (PMM) - v2: * Instead of sorting all qtests, leave the order intact and just filter the three affected architectures out. tests/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/Makefile b/tests/Makefile index 6086f68..b6470c8 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -162,7 +162,9 @@ check-qtest-microblazeel-y = $(check-qtest-microblaze-y) check-qtest-xtensaeb-y = $(check-qtest-xtensa-y) # qom-test works for all sysemu architectures: -$(foreach target,$(SYSEMU_TARGET_LIST), \ +QTEST_SYSEMU_TARGET_LIST=$(filter-out x86_64 microblazeel xtensaeb, \ +$(SYSEMU_TARGET_LIST)) +$(foreach target,$(QTEST_SYSEMU_TARGET_LIST), \ $(eval check-qtest-$(target)-y += tests/qom-test$(EXESUF))) check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ -- 1.8.4.5
Re: [Qemu-devel] [PATCH 28/35] pc: propagate memory hotplug event to ACPI device
On Mon, Apr 07, 2014 at 01:07:53PM +1000, Alexey Kardashevskiy wrote: On 04/05/2014 12:36 AM, Igor Mammedov wrote: Notify PIIX4_PM/ICH9LPC device about hotplug event, so that it would send SCI to guest notifying about newly added memory. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/pc.c | 13 + 1 file changed, 13 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 734c6ee..ee5cf88 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -60,6 +60,8 @@ #include acpi-build.h #include hw/mem/dimm.h #include trace.h +#include hw/acpi/piix4.h +#include hw/i386/ich9.h /* debug PC/ISA interrupts */ //#define DEBUG_IRQ @@ -1484,6 +1486,8 @@ void qemu_register_pc_machine(QEMUMachine *m) static void pc_dimm_plug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { +Object *acpi_dev; +HotplugHandlerClass *hhc; Error *local_err = NULL; PCMachineState *pcms = PC_MACHINE(hotplug_dev); DimmDevice *dimm = DIMM(dev); @@ -1517,10 +1521,19 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev, } trace_mhp_pc_dimm_assigned_slot(dimm-slot); +acpi_dev = (acpi_dev = piix4_pm_find()) ? acpi_dev : ich9_lpc_find(); wow. just wow. I had to read the C99 spec to find out if this was safe. :-) But I believe it is readable, I wouldn't mind keeping it that way. -- Eduardo
Re: [Qemu-devel] E820 (Re: [v4 PATCH 00/12] SMBIOS: build full tables in QEMU)
On Mon, Apr 07, 2014 at 09:09:56AM +0200, Gerd Hoffmann wrote: The only fly in this ointment may be that type 0 doesn't have a fixed length that could be edited in place, if you consider the various strings that get tacked on to the end of it. So you'd still have to slide the rest of the smbios payload left or right to shrink or enlarge the type 0 blob, depending on how you modify the various strings it contains... The dummy type 0 subtable that QEMU generates can have dummy space padded strings that the firmware can overwrite. Until recently, the max size smbios string was 64 bytes, so that size could be used. (As above, I admit that this is ugly, but the alternatives also seem ugly.) Another option would be to just leave the strings at a QEMU default as that's no different from what SeaBIOS does today. I don't think we need to make it that complicated. smbios tables don't have any references, right? I mean any references which would need a fixup (such as table pointers in RSDP in acpi) and therefore would need the romfile_loader. The string references within a table are relative don't need special care. The smbios anchor table needs to have the address of the main smbios table. It would be preferable to get the anchor table from qemu as the anchor table has the smbios version info. But, anchor table aside, you are correct. Gabriel has code to generate all tables needed in qemu meanwhile, so I think we can simply have a blob in fw_cfg with all tables (except type0). firmware generates type0 table like it does today, then simply appends the fw_cfg blob as-is, then appends a end-of-tables marker. Done. OVMF probably would have to parse the blob, split it into tables, then install them one by one. But I suspect that will be less code than dealing with the complex smbios fw_cfg interface we have today ... How about having QEMU produce the smbios table with a dummy type0 table and then both seabios and ovmf can replace the type0 table if desired. After all, if OVMF is splitting the blob into tables, it can just as easily replace type0 as append it. This way, the QEMU output is technically complete. And if someone wishes to code up SeaBIOS to do the type0 replace (I'm not convinced it's even necessary) then at least that SeaBIOS code could be used on coreboot as well. -Kevin
Re: [Qemu-devel] [SeaBIOS] [PATCH] hw/pci: reserve IO and mem for pci-2-pci bridges with no devices attached
On Mon, 2014-04-07 at 17:09 +0300, Michael S. Tsirkin wrote: On Mon, Apr 07, 2014 at 04:51:54PM +0300, Marcel Apfelbaum wrote: [...] I don't think we'll need that for the SHPC bridge. Why not? Because has shpc = not an PCIe port. (as far as I know) Anyway, why have shpc capability but no I/O or mem to support it? ^^^ [...] AFAIK the spec does not list reset value for this register. IIRC QEMU resets both to 0. Thanks, what do you think about the above ? ^^^ While I am not against it, it seems redundant. It has shpc = it needs I/O or mem space. Marcel - read back value value 0 means bridge does not support I/O. A similar trick should work for other optional resources. For express it indeed makes sense to avoid claiming IO address space. I'd try to find something more automatic though, where you don't need some kind of disable io for this express port config option. Won't same trick as above work? For express ports which can only have a single device underneath we can check whenever we have a device and if one is present already don't bother claiming extra resources for hotplug. +for (cap = pci_config_readb(pci-bdf, PCI_CAPABILITY_LIST); cap; +cap = pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_NEXT)) +if (pci_config_readb(pci-bdf, cap + PCI_CAP_LIST_ID) == cap_id) +return cap; I would also limit this to 256 iterations, to make sure we dont' get into an infinite loop with a broken device. Good point. cheers, Gerd
Re: [Qemu-devel] [PATCH v3 1/1] char/serial: Fix emptyness handling
On 4 April 2014 13:13, Peter Crosthwaite peter.crosthwa...@xilinx.com wrote: On Fri, Mar 28, 2014 at 10:10 PM, Paolo Bonzini pbonz...@redhat.com wrote: Il 28/03/2014 12:43, Don Slutz ha scritto: Ping. (Since this is a bug fix, I think it can go into 2.0) -Don Slutz I think the problem is that not many people understand the 8250 device model. CCing someone who hopefully does... I have a bit of experience with 16550 :) Ill push for a merge on this one. Applied to master, thanks. -- PMM
Re: [Qemu-devel] [PATCH 23/35] acpi:piix4: make plug/unlug callbacks generic
On Mon, 7 Apr 2014 16:25:30 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Apr 07, 2014 at 03:12:11PM +0200, Igor Mammedov wrote: On Mon, 7 Apr 2014 15:07:15 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Apr 07, 2014 at 02:00:37PM +0200, Igor Mammedov wrote: On Mon, 7 Apr 2014 14:32:41 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 04, 2014 at 03:36:48PM +0200, Igor Mammedov wrote: ... and report error if plugged in device is not supported. Later generic callbacks will be used by memory hotplug. Signed-off-by: Igor Mammedov imamm...@redhat.com OK in that case, how about teaching all hotplug callbacks about this? There are two ATM: shpc_device_hotplug_cb pcie_cap_slot_hotplug_cb Teach them both to fail gracefully if they get an object that is not a pci device. Afterwards, simply iterate over all objects of type TYPE_HOTPLUG_HANDLER and look for one that will accept your object. Then you would never know if any hotplug handler has actually handled event. Why not? Check the error. If no one accepts your object, return error to user. I think hotplug handler should return error if unsupported device passed in rather than ignore it. It makes catching wiring errors easier. Absolutely. Dropping error so that we could not care which hotplug handler should be notified, looks like a wrong direction and makes system more fragile. That's not what I was suggesting. It shouldn't be up to consumer to determine that event should be routed to it, but rather by external routing that knows what and when should be notified. Yes. So for each hotplug handler (err) handler-plug(device, err) if (!err) break; here it would break on the first handler that doesn't support device and tells so to caller. This is pseudo-code, I really mean !err == no error reported. * what if all handlers returned error, err might not reflect the actual error returned from handler that cares about device? * What if there would be more handlers that could or should handle event for device? * What if only some of compatible handler should handle event * What if handler should conditionally handle event and only caller knows about condition and have access to them? * What about ordering in which handlers should be called? Broadcast would be useful if it were impossible to know in advance which hotplug handler to use. Is there use case for this? And there isn't any routing here, it just blindly broadcast to every handler, regardless whether it's right or not. Yes - handlers verify what they can support. Sure handlers could verify, there is no harm in extra checking, but handlers should not decide what to handle. That's what I'm against from. It should be upto caller to decide if handler is the right one and call it. There shouldn't be a chance for random/wrong handler to be called. If broadcast should be ever done than it probably should be a part of DEVICE class and part of [Qemu-devel] [PATCH 08/35] qdev: hotplug for buss-less devices patch and be generic to all devices. Not sure what's suggested here. above looks like generic code that should be part of Device.realize() and should replace 08/35 patch if it's deemed as acceptable. I think implementing design like that requires much more though if viable and out of scope of this series. Andreas, since you care about QDEV do you have an opinion on ^^^ discussion? if (err) hotplug failed - destroy device --- hw/acpi/piix4.c | 31 ++- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index 67dc075..4341f82 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -310,19 +310,32 @@ static void piix4_pm_powerdown_req(Notifier *n, void *opaque) acpi_pm1_evt_power_down(s-ar); } -static void piix4_pci_device_plug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_plug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); -acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, errp); + +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { +acpi_pcihp_device_plug_cb(s-ar, s-irq, s-acpi_pci_hotplug, dev, + errp); +} else { +error_setg(errp, acpi: device plug request for not supported device +
Re: [Qemu-devel] [Spice-devel] [Xen-devel] Qemu 2.0 regression with xen: qemu crash on any domUs S.O. start
Il 07/04/2014 15:19, Fabio Fantoni ha scritto: Il 07/04/2014 12:20, Christophe Fergeau ha scritto: On Mon, Apr 07, 2014 at 11:59:06AM +0200, Fabio Fantoni wrote: Today I did some tests also with hvm and spice and I found another segfault with different backtrace to solve: (gdb) c Continuing. *Program received signal SIGSEGV, Segmentation fault.** **0x55855d30 in interface_client_monitors_config (sin=0x563b0260, ** **mc=0x0) at ui/spice-display.c:557** **557 if (mc-num_of_monitors 0) {* (gdb) bt full #0 0x55855d30 in interface_client_monitors_config ( sin=0x563b0260, mc=0x0) at ui/spice-display.c:557 ssd = 0x563b0210 info = {xoff = 0, yoff = 0, width = 0, height = 0} rc = 32767 __func__ = interface_client_monitors_config #1 0x74af5113 in ?? () from /usr/lib/x86_64-linux-gnu/libspice-server.so.1 No symbol table info available. A backtrace with spice-server debugging symbols installed would be helpful. Christophe Sorry, the -dbg for spice-server on official debian packages is missing, now I created and installed also the -dbg package and this is the new backtrace: (gdb) c Continuing. Program received signal SIGSEGV, Segmentation fault. 0x55855d30 in interface_client_monitors_config (sin=0x563b0260, mc=0x0) at ui/spice-display.c:557 557 if (mc-num_of_monitors 0) { (gdb) bt full #0 0x55855d30 in interface_client_monitors_config ( sin=0x563b0260, mc=0x0) at ui/spice-display.c:557 ssd = 0x563b0210 info = {xoff = 0, yoff = 0, width = 0, height = 0} rc = 32767 __func__ = interface_client_monitors_config #1 0x74af5113 in red_dispatcher_use_client_monitors_config () at red_dispatcher.c:318 now = 0x563b0300 #2 0x74ad87f5 in agent_msg_filter_process_data ( filter=filter@entry=0x562eb0c4, data=data@entry=0x7fffe0280128 \001, len=328, len@entry=348) at agent-msg-filter.c:95 msg_header = {protocol = optimized out, type = optimized out, opaque = optimized out, size = 328, data = 0x831fd4 Address 0x831fd4 out of bounds} __FUNCTION__ = agent_msg_filter_process_data #3 0x74b1af76 in reds_on_main_agent_data (mcc=0x56326e70, message=0x7fffe0280128, size=348) at reds.c:1117 dev_state = 0x562eb0a8 header = optimized out res = optimized out __FUNCTION__ = reds_on_main_agent_data #4 0x74ae989a in main_channel_handle_parsed (rcc=0x56326e70, size=optimized out, type=optimized out, message=0x7fffe0280128) ---Type return to continue, or q return to quit--- at main_channel.c:911 main_chan = 0x562ef2b0 mcc = 0x56326e70 __FUNCTION__ = main_channel_handle_parsed #5 0x74aee470 in red_peer_handle_incoming (handler=0x5632af80, stream=0x565adba0) at red_channel.c:287 ret_handle = optimized out bytes_read = optimized out msg_type = 107 parsed = optimized out parsed_free = 0x74ba8620 nofree msg_size = 348 #6 red_channel_client_receive (rcc=rcc@entry=0x56326e70) at red_channel.c:309 No locals. #7 0x74af0d8c in red_channel_client_event (fd=optimized out, event=optimized out, data=0x56326e70) at red_channel.c:1435 rcc = 0x56326e70 #8 0x55851f82 in watch_read (opaque=0x5666e0a0) at ui/spice-core.c:101 watch = 0x5666e0a0 #9 0x557ce1f8 in qemu_iohandler_poll (pollfds=0x562e8e00, ret=1) at iohandler.c:143 revents = 1 pioh = 0x5634e080 ---Type return to continue, or q return to quit--- ioh = 0x5632fa30 #10 0x557cf2a4 in main_loop_wait (nonblocking=0) at main-loop.c:485 ret = 1 timeout = 4294967295 timeout_ns = 4237075 #11 0x5587acd8 in main_loop () at vl.c:2051 nonblocking = false last_io = 1 #12 0x558826b2 in main (argc=36, argv=0x7fffe358, envp=0x7fffe480) at vl.c:4507 i = 64 snapshot = 0 linux_boot = 0 icount_option = 0x0 initrd_filename = 0x0 kernel_filename = 0x0 kernel_cmdline = 0x55a1b5c4 boot_order = 0x562e7ee0 dc ds = 0x563d8fd0 cyls = 0 heads = 0 secs = 0 translation = 0 hda_opts = 0x0 opts = 0x562e7e30 ---Type return to continue, or q return to quit--- machine_opts = 0x562e84b0 olist = 0x55e00e00 optind = 36 optarg = 0x7fffe915 if=ide,index=1,media=cdrom,cache=writeback,id=ide-832 loadvm = 0x0 machine_class = 0x562e02a0 machine = 0x55e067e0 cpu_model = 0x0 vga_model = 0x0 qtest_chrdev = 0x0 qtest_log = 0x0 pid_file = 0x0 incoming = 0x0
Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/3] int128.h: Avoid undefined behaviours involving signed arithmetic
On 04/06/2014 08:27 AM, Peter Maydell wrote: (if C compilers had a behave like a sane 2s complement system for signed arithmetic option I'd be advocating for us using it...) -fwrapv. r~
Re: [Qemu-devel] [PATCH v3 24/26] tcg-aarch64: Replace aarch64_ldst_op_data with AArch64LdstType
On 04/07/2014 04:45 AM, Claudio Fontana wrote: On 03.04.2014 21:56, Richard Henderson wrote: The definition of op_type wasn't encoded for the proper shift for the field, making the implementations confusing. Signed-off-by: Richard Henderson r...@twiddle.net At the end of the day the magic values remain in the load/store instructions though. Can we find a way to replace them with INSN_-something like for the others? I think I was doing something of the sort in a now obsolete patch I suggested some time early this year, see if it helps: http://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05074.html Yes, we can. I'll do something for v3, Claudio --- tcg/aarch64/tcg-target.c | 42 +- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c index 9a2e4a6..a538a87 100644 --- a/tcg/aarch64/tcg-target.c +++ b/tcg/aarch64/tcg-target.c @@ -242,12 +242,12 @@ static const enum aarch64_cond_code tcg_cond_to_aarch64[] = { [TCG_COND_LEU] = COND_LS, }; -enum aarch64_ldst_op_type { /* type of operation */ -LDST_ST = 0x0,/* store */ -LDST_LD = 0x4,/* load */ -LDST_LD_S_X = 0x8, /* load and sign-extend into Xt */ -LDST_LD_S_W = 0xc, /* load and sign-extend into Wt */ -}; +typedef enum { +LDST_ST = 0,/* store */ +LDST_LD = 1,/* load */ +LDST_LD_S_X = 2, /* load and sign-extend into Xt */ +LDST_LD_S_W = 3, /* load and sign-extend into Wt */ +} AArch64LdstType; /* We encode the format of the insn into the beginning of the name, so that we can have the preprocessor help typecheck the insn vs the output @@ -483,22 +483,19 @@ static void tcg_out_insn_3509(TCGContext *s, AArch64Insn insn, TCGType ext, } -static inline void tcg_out_ldst_9(TCGContext *s, TCGMemOp size, - enum aarch64_ldst_op_type op_type, - TCGReg rd, TCGReg rn, intptr_t offset) +static void tcg_out_ldst_9(TCGContext *s, TCGMemOp size, AArch64LdstType type, + TCGReg rd, TCGReg rn, intptr_t offset) { /* use LDUR with BASE register with 9bit signed unscaled offset */ -tcg_out32(s, 0x3800 | size 30 | op_type 20 +tcg_out32(s, 0x3800 | size 30 | type 22 | (offset 0x1ff) 12 | rn 5 | rd); } /* tcg_out_ldst_12 expects a scaled unsigned immediate offset */ -static inline void tcg_out_ldst_12(TCGContext *s, TCGMemOp size, - enum aarch64_ldst_op_type op_type, - TCGReg rd, TCGReg rn, - tcg_target_ulong scaled_uimm) +static void tcg_out_ldst_12(TCGContext *s, TCGMemOp size, AArch64LdstType type, +TCGReg rd, TCGReg rn, tcg_target_ulong scaled_uimm) { -tcg_out32(s, 0x3900 | size 30 | op_type 20 +tcg_out32(s, 0x3900 | size 30 | type 22 | scaled_uimm 10 | rn 5 | rd); } @@ -637,21 +634,16 @@ static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg rd, } } -static inline void tcg_out_ldst_r(TCGContext *s, TCGMemOp size, - enum aarch64_ldst_op_type op_type, - TCGReg rd, TCGReg base, TCGReg regoff) +static void tcg_out_ldst_r(TCGContext *s, TCGMemOp size, AArch64LdstType type, + TCGReg rd, TCGReg base, TCGReg regoff) { -/* load from memory to register using base + 64bit register offset */ -/* using f.e. STR Wt, [Xn, Xm] 0xb8600800|(regoff 16)|(base 5)|rd */ -/* the 0x6000 is for the no extend field */ -tcg_out32(s, 0x38206800 | size 30 | op_type 20 +tcg_out32(s, 0x38206800 | size 30 | type 22 | regoff 16 | base 5 | rd); } /* solve the whole ldst problem */ -static inline void tcg_out_ldst(TCGContext *s, TCGMemOp size, -enum aarch64_ldst_op_type type, -TCGReg rd, TCGReg rn, intptr_t offset) +static void tcg_out_ldst(TCGContext *s, TCGMemOp size, AArch64LdstType type, + TCGReg rd, TCGReg rn, intptr_t offset) { if (offset = -256 offset 256) { tcg_out_ldst_9(s, size, type, rd, rn, offset);
Re: [Qemu-devel] [PATCH 28/35] pc: propagate memory hotplug event to ACPI device
On Mon, 7 Apr 2014 13:23:54 +0300 Michael S. Tsirkin m...@redhat.com wrote: On Fri, Apr 04, 2014 at 03:36:53PM +0200, Igor Mammedov wrote: Notify PIIX4_PM/ICH9LPC device about hotplug event, so that it would send SCI to guest notifying about newly added memory. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/pc.c | 13 + 1 file changed, 13 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 734c6ee..ee5cf88 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -60,6 +60,8 @@ #include acpi-build.h #include hw/mem/dimm.h #include trace.h +#include hw/acpi/piix4.h +#include hw/i386/ich9.h /* debug PC/ISA interrupts */ //#define DEBUG_IRQ @@ -1484,6 +1486,8 @@ void qemu_register_pc_machine(QEMUMachine *m) static void pc_dimm_plug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { +Object *acpi_dev; +HotplugHandlerClass *hhc; Error *local_err = NULL; PCMachineState *pcms = PC_MACHINE(hotplug_dev); DimmDevice *dimm = DIMM(dev); @@ -1517,10 +1521,19 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev, } trace_mhp_pc_dimm_assigned_slot(dimm-slot); +acpi_dev = (acpi_dev = piix4_pm_find()) ? acpi_dev : ich9_lpc_find(); +if (!acpi_dev) { +error_setg(local_err, + memory hotplug is not enabled: missing acpi device); +return; +} + +hhc = HOTPLUG_HANDLER_GET_CLASS(acpi_dev); How about simply looking for a hotplug handler type device instead? We aren't likely to have many of these, are we? How about adding linkacpi_device to PCMachine when it's created and use it instead of piix4_pm_find()/ich9_lpc_find() everywhere? that would allow to remove above searches in QOM tree and simplify code including acpi-build.c memory_region_add_subregion(pcms-hotplug_memory, addr - pcms-hotplug_memory_base, mr); vmstate_register_ram(mr, dev); +hhc-plug(HOTPLUG_HANDLER(acpi_dev), dev, local_err); out: error_propagate(errp, local_err); -- 1.9.0
Re: [Qemu-devel] E820 (Re: [v4 PATCH 00/12] SMBIOS: build full tables in QEMU)
On 04/07/14 16:14, Kevin O'Connor wrote: On Mon, Apr 07, 2014 at 09:09:56AM +0200, Gerd Hoffmann wrote: The only fly in this ointment may be that type 0 doesn't have a fixed length that could be edited in place, if you consider the various strings that get tacked on to the end of it. So you'd still have to slide the rest of the smbios payload left or right to shrink or enlarge the type 0 blob, depending on how you modify the various strings it contains... The dummy type 0 subtable that QEMU generates can have dummy space padded strings that the firmware can overwrite. Until recently, the max size smbios string was 64 bytes, so that size could be used. (As above, I admit that this is ugly, but the alternatives also seem ugly.) Another option would be to just leave the strings at a QEMU default as that's no different from what SeaBIOS does today. I don't think we need to make it that complicated. smbios tables don't have any references, right? I mean any references which would need a fixup (such as table pointers in RSDP in acpi) and therefore would need the romfile_loader. The string references within a table are relative don't need special care. The smbios anchor table needs to have the address of the main smbios table. It would be preferable to get the anchor table from qemu as the anchor table has the smbios version info. But, anchor table aside, you are correct. Gabriel has code to generate all tables needed in qemu meanwhile, so I think we can simply have a blob in fw_cfg with all tables (except type0). firmware generates type0 table like it does today, then simply appends the fw_cfg blob as-is, then appends a end-of-tables marker. Done. OVMF probably would have to parse the blob, split it into tables, then install them one by one. But I suspect that will be less code than dealing with the complex smbios fw_cfg interface we have today ... How about having QEMU produce the smbios table with a dummy type0 table and then both seabios and ovmf can replace the type0 table if desired. After all, if OVMF is splitting the blob into tables, it can just as easily replace type0 as append it. This way, the QEMU output is technically complete. And if someone wishes to code up SeaBIOS to do the type0 replace (I'm not convinced it's even necessary) then at least that SeaBIOS code could be used on coreboot as well. Works for me. Thanks Laszlo
Re: [Qemu-devel] [PATCH 28/35] pc: propagate memory hotplug event to ACPI device
On Mon, 7 Apr 2014 11:13:01 -0300 Eduardo Habkost ehabk...@redhat.com wrote: On Mon, Apr 07, 2014 at 01:07:53PM +1000, Alexey Kardashevskiy wrote: On 04/05/2014 12:36 AM, Igor Mammedov wrote: Notify PIIX4_PM/ICH9LPC device about hotplug event, so that it would send SCI to guest notifying about newly added memory. Signed-off-by: Igor Mammedov imamm...@redhat.com --- hw/i386/pc.c | 13 + 1 file changed, 13 insertions(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 734c6ee..ee5cf88 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -60,6 +60,8 @@ #include acpi-build.h #include hw/mem/dimm.h #include trace.h +#include hw/acpi/piix4.h +#include hw/i386/ich9.h /* debug PC/ISA interrupts */ //#define DEBUG_IRQ @@ -1484,6 +1486,8 @@ void qemu_register_pc_machine(QEMUMachine *m) static void pc_dimm_plug(HotplugHandler *hotplug_dev, DeviceState *dev, Error **errp) { +Object *acpi_dev; +HotplugHandlerClass *hhc; Error *local_err = NULL; PCMachineState *pcms = PC_MACHINE(hotplug_dev); DimmDevice *dimm = DIMM(dev); @@ -1517,10 +1521,19 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev, } trace_mhp_pc_dimm_assigned_slot(dimm-slot); +acpi_dev = (acpi_dev = piix4_pm_find()) ? acpi_dev : ; ; wow. just wow. I had to read the C99 spec to find out if this was safe. :-) But I believe it is readable, I wouldn't mind keeping it that way. I'll change it to a less obscure form: acpi_dev = piix4_pm_find(); if (!acpi_dev) { acpi_dev = ich9_lpc_find(); }
Re: [Qemu-devel] [Qemu-trivial] [PATCH 2/3] int128.h: Avoid undefined behaviours involving signed arithmetic
On 7 April 2014 15:25, Richard Henderson r...@twiddle.net wrote: On 04/06/2014 08:27 AM, Peter Maydell wrote: (if C compilers had a behave like a sane 2s complement system for signed arithmetic option I'd be advocating for us using it...) -fwrapv. Well, we should use that then :-) thanks -- PMM