Re: [Qemu-devel] [PATCH v8] block/raw-posix.c: Fix raw_getlength() on Mac OS X for CD
Programmingkid programmingk...@gmail.com writes:
Subject was:
Re: [PATCH v7] block/raw-posix.c: Fixes raw_getlength()
on Mac OS X so that it reports the correct length of a real CD
Patch history information goes...
This patch allows Mac OS X to use a real CDROM disc in QEMU.
Testing this patch will require using QEMU v2.2.0 because the
current git version has a bug in it that prevents /dev/cdrom from
being used. make check did pass and my Debian boot disc did work.
Signed-off-by: John Arbuckle programmingk...@gmail.com
---
... below the --- divider.
Fixed code indentation to be inline with removed
size = LLONG_MAX.
block/raw-posix.c | 15 ++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index e51293a..fa431b2 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1312,7 +1312,20 @@ again:
if (size == 0)
#endif
#if defined(__APPLE__) defined(__MACH__)
-size = LLONG_MAX;
+{
+uint64_t sectors = 0;
+uint32_t sector_size = 0;
Ignorant question: why do you need to initialize these?
Patch looks plausible otherwise, but know nothing about Macs :)
+
+if (ioctl(fd, DKIOCGETBLOCKCOUNT, sectors) == 0
+ioctl(fd, DKIOCGETBLOCKSIZE, sector_size) == 0) {
+size = sectors * sector_size;
+} else {
+size = lseek(fd, 0LL, SEEK_END);
+if (size 0) {
+return -errno;
+}
+}
+}
#else
size = lseek(fd, 0LL, SEEK_END);
if (size 0) {
Re: [Qemu-devel] [RFC 42/47] acpi: make tables linker-loader available to other targets
On Tue, 20 Jan 2015 00:05:15 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Fri, Dec 19, 2014 at 02:02:37AM +, Igor Mammedov wrote:
keeping bios-linker-loader.c i386 specific would break build
of mips target which is built with CONFIG_ACPI which would
dependend on it in following patch that adds acpi_def_block()
term. Also UEFI for ARM target is going to use linker as well
so it makes sense to move it into generic target independent
ACPI part.
Signed-off-by: Igor Mammedov imamm...@redhat.com
I reworked this one to avoid the huge diffs.
Pls check the pci branch.
Thanks.
---
hw/acpi/Makefile.objs| 2 +-
hw/acpi/acpi_gen_utils.c | 1 +
hw/acpi/bios-linker-loader.c | 157
+++
hw/i386/Makefile.objs| 1 -
hw/i386/acpi-build.c | 2 +-
hw/i386/bios-linker-loader.c | 157
---
hw/i386/bios-linker-loader.h | 27 --
include/hw/acpi/bios-linker-loader.h | 27 ++
8 files changed, 187 insertions(+), 187 deletions(-)
create mode 100644 hw/acpi/bios-linker-loader.c
delete mode 100644 hw/i386/bios-linker-loader.c
delete mode 100644 hw/i386/bios-linker-loader.h
create mode 100644 include/hw/acpi/bios-linker-loader.h
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 4237232..02ca4ed 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -1,4 +1,4 @@
common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o
common-obj-$(CONFIG_ACPI) += memory_hotplug.o
common-obj-$(CONFIG_ACPI) += acpi_interface.o
-common-obj-$(CONFIG_ACPI) += acpi_gen_utils.o
+common-obj-$(CONFIG_ACPI) += acpi_gen_utils.o bios-linker-loader.o
diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
index 39cbab7..cc2fa03 100644
--- a/hw/acpi/acpi_gen_utils.c
+++ b/hw/acpi/acpi_gen_utils.c
@@ -5,6 +5,7 @@
#include string.h
#include hw/acpi/acpi_gen_utils.h
#include qemu/bswap.h
+#include hw/acpi/bios-linker-loader.h
GArray *build_alloc_array(void)
{
diff --git a/hw/acpi/bios-linker-loader.c b/hw/acpi/bios-linker-loader.c
new file mode 100644
index 000..5cc4d90
--- /dev/null
+++ b/hw/acpi/bios-linker-loader.c
@@ -0,0 +1,157 @@
+/* Dynamic linker/loader of ACPI tables
+ *
+ * Copyright (C) 2013 Red Hat Inc
+ *
+ * Author: Michael S. Tsirkin m...@redhat.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see http://www.gnu.org/licenses/.
+ */
+
+#include qemu-common.h
+#include hw/acpi/bios-linker-loader.h
+#include hw/nvram/fw_cfg.h
+
+#include qemu/bswap.h
+
+#define BIOS_LINKER_LOADER_FILESZ FW_CFG_MAX_FILE_PATH
+
+struct BiosLinkerLoaderEntry {
+uint32_t command;
+union {
+/*
+ * COMMAND_ALLOCATE - allocate a table from @alloc.file
+ * subject to @alloc.align alignment (must be power of 2)
+ * and @alloc.zone (can be HIGH or FSEG) requirements.
+ *
+ * Must appear exactly once for each file, and before
+ * this file is referenced by any other command.
+ */
+struct {
+char file[BIOS_LINKER_LOADER_FILESZ];
+uint32_t align;
+uint8_t zone;
+} alloc;
+
+/*
+ * COMMAND_ADD_POINTER - patch the table (originating from
+ * @dest_file) at @pointer.offset, by adding a pointer to the table
+ * originating from @src_file. 1,2,4 or 8 byte unsigned
+ * addition is used depending on @pointer.size.
+ */
+struct {
+char dest_file[BIOS_LINKER_LOADER_FILESZ];
+char src_file[BIOS_LINKER_LOADER_FILESZ];
+uint32_t offset;
+uint8_t size;
+} pointer;
+
+/*
+ * COMMAND_ADD_CHECKSUM - calculate checksum of the range
specified by
+ * @cksum_start and @cksum_length fields,
+ * and then add the value at @cksum.offset.
+ * Checksum simply sums -X for each byte X in the range
+ * using 8-bit math.
+ */
+struct {
+char file[BIOS_LINKER_LOADER_FILESZ];
+uint32_t offset;
+uint32_t start;
+
Re: [Qemu-devel] [v6][PATCH 05/10] xen, gfx passthrough: basic graphics passthrough support
On Di, 2015-01-20 at 11:14 +0800, Chen, Tiejun wrote: On 2015/1/19 19:45, Gerd Hoffmann wrote: On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote: +DEF(gfx_passthru, 0, QEMU_OPTION_gfx_passthru, +-gfx_passthru enable Intel IGD passthrough by XEN\n, +QEMU_ARCH_ALL) +STEXI +@item -gfx_passthru +@findex -gfx_passthru +Enable Intel IGD passthrough by XEN +ETEXI Make that a machine option, i.e. -machine pc,igd-passthru=on? Yeah but I think we need -machine xenfv,igd-passthru=on here. IIRC xen decided to stop using xenfv and use pc-$version instead (with $version being what was current at release time, 1.6 for xen 4.4 I think, to avoid surprises like the address space layout changes in more recent machine types). cheers, Gerd
Re: [Qemu-devel] [PATCH v3 7/8] acpi: drop min-bytes in build_package()
Reviewed-by: Claudio Fontana claudio.font...@huawei.com
On 19.12.2014 12:47, Igor Mammedov wrote:
Signed-off-by: Igor Mammedov imamm...@redhat.com
---
hw/acpi/acpi_gen_utils.c | 14 --
hw/i386/acpi-build.c | 13 ++---
include/hw/acpi/acpi_gen_utils.h | 4 ++--
3 files changed, 12 insertions(+), 19 deletions(-)
diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
index d5fca8e..eee8066 100644
--- a/hw/acpi/acpi_gen_utils.c
+++ b/hw/acpi/acpi_gen_utils.c
@@ -146,7 +146,7 @@ enum {
PACKAGE_LENGTH_4BYTE_SHIFT = 20,
};
-void build_prepend_package_length(GArray *package, unsigned min_bytes)
+void build_prepend_package_length(GArray *package)
{
uint8_t byte;
unsigned length = package-len;
@@ -162,11 +162,6 @@ void build_prepend_package_length(GArray *package,
unsigned min_bytes)
length_bytes = 4;
}
-/* Force length to at least min_bytes.
- * This wastes memory but that's how bios did it.
- */
-length_bytes = MAX(length_bytes, min_bytes);
-
/* PkgLength is the length of the inclusive length of the data. */
length += length_bytes;
@@ -199,15 +194,15 @@ void build_prepend_package_length(GArray *package,
unsigned min_bytes)
build_prepend_byte(package, byte);
}
-void build_package(GArray *package, uint8_t op, unsigned min_bytes)
+void build_package(GArray *package, uint8_t op)
{
-build_prepend_package_length(package, min_bytes);
+build_prepend_package_length(package);
build_prepend_byte(package, op);
}
void build_extop_package(GArray *package, uint8_t op)
{
-build_package(package, op, 1);
+build_package(package, op);
build_prepend_byte(package, 0x5B); /* ExtOpPrefix */
}
@@ -251,4 +246,3 @@ void build_append_int(GArray *table, uint32_t value)
build_append_value(table, value, 4);
}
}
-
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 7642f6d..94202b5 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -289,7 +289,7 @@ static void build_append_and_cleanup_method(GArray
*device, GArray *method)
{
uint8_t op = 0x14; /* MethodOp */
-build_package(method, op, 0);
+build_package(method, op);
build_append_array(device, method);
build_free_array(method);
@@ -310,7 +310,7 @@ static void build_append_notify_target_ifequal(GArray
*method,
build_append_byte(notify, 0x69); /* Arg1Op */
/* Pack it up */
-build_package(notify, op, 1);
+build_package(notify, op);
build_append_array(method, notify);
@@ -823,7 +823,7 @@ static void build_pci_bus_end(PCIBus *bus, void
*bus_state)
build_append_byte(notify, 0x69); /* Arg1Op */
/* Pack it up */
-build_package(notify, op, 0);
+build_package(notify, op);
build_append_array(method, notify);
@@ -864,7 +864,7 @@ static void build_pci_bus_end(PCIBus *bus, void
*bus_state)
if (bus-parent_dev) {
build_extop_package(bus_table, op);
} else {
-build_package(bus_table, op, 0);
+build_package(bus_table, op);
}
/* Append our bus description to parent table */
@@ -987,7 +987,7 @@ build_ssdt(GArray *table_data, GArray *linker,
build_append_byte(package, b);
}
-build_package(package, op, 2);
+build_package(package, op);
build_append_array(sb_scope, package);
build_free_array(package);
}
@@ -1035,8 +1035,7 @@ build_ssdt(GArray *table_data, GArray *linker,
build_append_array(sb_scope, hotplug_state.device_table);
build_pci_bus_state_cleanup(hotplug_state);
}
-
-build_package(sb_scope, op, 3);
+build_package(sb_scope, op);
build_append_array(table_data, sb_scope);
build_free_array(sb_scope);
}
diff --git a/include/hw/acpi/acpi_gen_utils.h
b/include/hw/acpi/acpi_gen_utils.h
index fd50625..199f003 100644
--- a/include/hw/acpi/acpi_gen_utils.h
+++ b/include/hw/acpi/acpi_gen_utils.h
@@ -14,8 +14,8 @@ void build_append_array(GArray *array, GArray *val);
void GCC_FMT_ATTR(2, 3)
build_append_namestring(GArray *array, const char *format, ...);
-void build_prepend_package_length(GArray *package, unsigned min_bytes);
-void build_package(GArray *package, uint8_t op, unsigned min_bytes);
+void build_prepend_package_length(GArray *package);
+void build_package(GArray *package, uint8_t op);
void build_append_value(GArray *table, uint32_t value, int size);
void build_append_int(GArray *table, uint32_t value);
void build_extop_package(GArray *package, uint8_t op);
--
Claudio Fontana
Server Virtualization Architect
Huawei Technologies Duesseldorf GmbH
Riesstraße 25 - 80992 München
office: +49 89 158834 4135
[Qemu-devel] [PATCH v2 1/1] Print PID and time in stderr traces
From: Dr. David Alan Gilbert dgilb...@redhat.com
When debugging migration it's useful to know the PID of
each trace message so you can figure out if it came from the source
or the destination.
Printing the time makes it easy to do latency measurements or timings
between trace points.
Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
---
v2
0 pad usec part
scripts/tracetool/backend/stderr.py | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/scripts/tracetool/backend/stderr.py
b/scripts/tracetool/backend/stderr.py
index 2a1e906..ca58054 100644
--- a/scripts/tracetool/backend/stderr.py
+++ b/scripts/tracetool/backend/stderr.py
@@ -21,6 +21,9 @@ PUBLIC = True
def generate_h_begin(events):
out('#include stdio.h',
+'#include sys/time.h',
+'#include sys/types.h',
+'#include unistd.h',
'#include trace/control.h',
'')
@@ -31,7 +34,12 @@ def generate_h(event):
argnames = , + argnames
out('if (trace_event_get_state(%(event_id)s)) {',
-'fprintf(stderr, %(name)s %(fmt)s \\n %(argnames)s);',
+'struct timeval _now;',
+'gettimeofday(_now, NULL);',
+'fprintf(stderr, %%d@%%zd.%%06zd:%(name)s %(fmt)s \\n,',
+'getpid(),',
+'(size_t)_now.tv_sec, (size_t)_now.tv_usec',
+'%(argnames)s);',
'}',
event_id=TRACE_ + event.name.upper(),
name=event.name,
--
2.1.0
Re: [Qemu-devel] [RFC 41/47] pc: acpi-build: create PCI0._CRS dynamically
On Mon, 19 Jan 2015 23:55:37 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Jan 19, 2015 at 01:42:25PM +0100, Paolo Bonzini wrote: On 19/12/2014 03:02, Igor Mammedov wrote: Replace template patching and runtime calculation in _CRS() method with static _CRS defined in SSDT. It also drops manual hole patching for reserved PCI/MEM/CPU hoptlug MMIO resources and utilizes the fact that MMIO resources are reserved by respective child /i.e. PHPR, MHPD, PRES/ containers. Signed-off-by: Igor Mammedov imamm...@redhat.com This is a good idea. It's simpler to just do this in the SSDT than to split it between DSDT and SSDT. The AML's purpose is just to build a static value anyway. Paolo I think Marcel has patches going in exactly the reverse direction with this. From my talk with him, he was doing/needs a similar thing. So this was simplifying his job. Marcel?
[Qemu-devel] [PATCH] qdev: Don't exit when running into bad -global
-global lets you set a nice booby-trap for yourself:
$ qemu-system-x86_64 -nodefaults -S -display none -usb -monitor stdio
-global usb-mouse.usb_version=l
QEMU 2.1.94 monitor - type 'help' for more information
(qemu) device_add usb-mouse
Parameter 'usb_version' expects an int64 value or range
$ echo $?
1
Not nice. Until commit 3196270 we even abort()ed.
The same error triggers if you manage to screw up a machine type's
compat_props. To demonstrate, change HW_COMPAT_2_1's entry to
.driver = usb-mouse,\
.property = usb_version,\
.value= 1, \
Then run
$ qemu-system-x86_64 -usb -M pc-i440fx-2.1 -device usb-mouse
upstream-qemu: -device usb-mouse: Parameter 'usb_version' expects an int64
value or range
$ echo $?
1
One of our creatively cruel error messages.
Since this is actually a coding error, we *should* abort() here.
Replace the error by an assertion failure in this case.
But turn the fatal error into a mere warning when the faulty
GlobalProperty comes from the user. Looks like this:
$ qemu-system-x86_64 -nodefaults -S -display none -usb -monitor stdio
-global usb-mouse.usb_version=l
QEMU 2.1.94 monitor - type 'help' for more information
(qemu) device_add usb-mouse
Warning: global usb-mouse.usb_version=l ignored (Parameter 'usb_version'
expects an int64 value or range)
(qemu)
This is consistent with how we handle similarly unusable -global in
qdev_prop_check_globals().
You could argue that the error should make device_add fail. Would be
harder, because we're running within TypeInfo's instance_post_init()
method device_post_init(), which can't fail.
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/core/qdev-properties.c| 21 +
hw/core/qdev.c | 8 +---
include/hw/qdev-properties.h | 4 +---
3 files changed, 11 insertions(+), 22 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 2e47f70..5a4e4d5 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -990,8 +990,8 @@ int qdev_prop_check_globals(void)
return ret;
}
-void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename,
-Error **errp)
+static void qdev_prop_set_globals_for_type(DeviceState *dev,
+const char *typename)
{
GlobalProperty *prop;
@@ -1004,25 +1004,22 @@ void qdev_prop_set_globals_for_type(DeviceState *dev,
const char *typename,
prop-used = true;
object_property_parse(OBJECT(dev), prop-value, prop-property, err);
if (err != NULL) {
-error_propagate(errp, err);
+assert(prop-user_provided);
+error_report(Warning: global %s.%s=%s ignored (%s),
+ prop-driver, prop-property, prop-value,
+ error_get_pretty(err));
+error_free(err);
return;
}
}
}
-void qdev_prop_set_globals(DeviceState *dev, Error **errp)
+void qdev_prop_set_globals(DeviceState *dev)
{
ObjectClass *class = object_get_class(OBJECT(dev));
do {
-Error *err = NULL;
-
-qdev_prop_set_globals_for_type(dev, object_class_get_name(class),
- err);
-if (err != NULL) {
-error_propagate(errp, err);
-return;
-}
+qdev_prop_set_globals_for_type(dev, object_class_get_name(class));
class = object_class_get_parent(class);
} while (class);
}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 901f289..827c084 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -1126,13 +1126,7 @@ static void device_initfn(Object *obj)
static void device_post_init(Object *obj)
{
-Error *err = NULL;
-qdev_prop_set_globals(DEVICE(obj), err);
-if (err) {
-qerror_report_err(err);
-error_free(err);
-exit(EXIT_FAILURE);
-}
+qdev_prop_set_globals(DEVICE(obj));
}
/* Unlink device from bus and free the structure. */
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 070006c..57ee363 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -180,9 +180,7 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name,
void *value);
void qdev_prop_register_global(GlobalProperty *prop);
void qdev_prop_register_global_list(GlobalProperty *props);
int qdev_prop_check_globals(void);
-void qdev_prop_set_globals(DeviceState *dev, Error **errp);
-void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename,
-Error **errp);
+void qdev_prop_set_globals(DeviceState *dev);
void error_set_from_qdev_prop_error(Error **errp, int ret, DeviceState *dev,
Property *prop, const char *value);
--
1.9.3
Re: [Qemu-devel] [PATCH v11 03/13] qmp: Add block-dirty-bitmap-add and block-dirty-bitmap-remove
John Snow js...@redhat.com writes:
On 01/19/2015 05:08 AM, Markus Armbruster wrote:
John Snow js...@redhat.com writes:
On 01/16/2015 10:36 AM, Max Reitz wrote:
On 2015-01-12 at 11:30, John Snow wrote:
From: Fam Zheng f...@redhat.com
The new command pair is added to manage user created dirty bitmap. The
dirty bitmap's name is mandatory and must be unique for the same device,
but different devices can have bitmaps with the same names.
The granularity is an optional field. If it is not specified, we will
choose a default granularity based on the cluster size if available,
clamped to between 4K and 64K to mirror how the 'mirror' code was
already choosing granularity. If we do not have cluster size info
available, we choose 64K. This code has been factored out into a helper
shared with block/mirror.
This patch also introduces the 'block_dirty_bitmap_lookup' helper,
which takes a device name and a dirty bitmap name and validates the
lookup, returning NULL and setting errp if there is a problem with
either field. This helper will be re-used in future patches in this
series.
The types added to block-core.json will be re-used in future patches
in this series, see:
'qapi: Add transaction support to block-dirty-bitmap-{add, enable,
disable}'
Signed-off-by: Fam Zheng f...@redhat.com
Signed-off-by: John Snow js...@redhat.com
---
block.c | 20 ++
block/mirror.c| 10 +
blockdev.c| 100
++
include/block/block.h | 1 +
qapi/block-core.json | 55 +++
qmp-commands.hx | 51 +
6 files changed, 228 insertions(+), 9 deletions(-)
diff --git a/block.c b/block.c
index bfeae6b..3eb77ee 100644
--- a/block.c
+++ b/block.c
@@ -5417,6 +5417,26 @@ int bdrv_get_dirty(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, int64_t sector
}
}
+/**
+ * Chooses a default granularity based on the existing cluster size,
+ * but clamped between [4K, 64K]. Defaults to 64K in the case that there
+ * is no cluster size information available.
+ */
+uint64_t bdrv_get_default_bitmap_granularity(BlockDriverState *bs)
+{
+BlockDriverInfo bdi;
+uint64_t granularity;
+
+if (bdrv_get_info(bs, bdi) = 0 bdi.cluster_size != 0) {
+granularity = MAX(4096, bdi.cluster_size);
+granularity = MIN(65536, granularity);
+} else {
+granularity = 65536;
+}
+
+return granularity;
+}
+
void bdrv_dirty_iter_init(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap, HBitmapIter *hbi)
{
diff --git a/block/mirror.c b/block/mirror.c
index d819952..fc545f1 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -667,15 +667,7 @@ static void mirror_start_job(BlockDriverState
*bs, BlockDriverState *target,
MirrorBlockJob *s;
if (granularity == 0) {
-/* Choose the default granularity based on the target file's
cluster
- * size, clamped between 4k and 64k. */
-BlockDriverInfo bdi;
-if (bdrv_get_info(target, bdi) = 0 bdi.cluster_size != 0) {
-granularity = MAX(4096, bdi.cluster_size);
-granularity = MIN(65536, granularity);
-} else {
-granularity = 65536;
-}
+granularity = bdrv_get_default_bitmap_granularity(target);
}
assert ((granularity (granularity - 1)) == 0);
diff --git a/blockdev.c b/blockdev.c
index 5651a8e..95251c7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1173,6 +1173,48 @@ out_aio_context:
return NULL;
}
+/**
+ * Return a dirty bitmap (if present), after validating
+ * the node reference and bitmap names. Returns NULL on error,
+ * including when the BDS and/or bitmap is not found.
+ */
+static BdrvDirtyBitmap *block_dirty_bitmap_lookup(const char *node_ref,
+ const char *name,
+ BlockDriverState
**pbs,
+ Error **errp)
+{
+BlockDriverState *bs;
+BdrvDirtyBitmap *bitmap;
+
+if (!node_ref) {
+error_setg(errp, Node reference cannot be NULL);
+return NULL;
+}
+if (!name) {
+error_setg(errp, Bitmap name cannot be NULL);
+return NULL;
+}
+
+bs = bdrv_lookup_bs(node_ref, node_ref, errp);
+if (!bs) {
+error_setg(errp, Node reference '%s' not found, node_ref);
No need to throw the (hopefully) perfectly fine Error code returned by
bdrv_lookup_bs() away.
I just wanted an error message consistent with the parameter name, in
this case. i.e., We couldn't find the Node reference instead of
device or node name. Just trying to distinguish the fact that this
is an arbitrary reference in the error message.
I can still remove it, but I am curious to see what Markus
Re: [Qemu-devel] [RFC 41/47] pc: acpi-build: create PCI0._CRS dynamically
On 01/19/2015 11:55 PM, Michael S. Tsirkin wrote: On Mon, Jan 19, 2015 at 01:42:25PM +0100, Paolo Bonzini wrote: On 19/12/2014 03:02, Igor Mammedov wrote: Replace template patching and runtime calculation in _CRS() method with static _CRS defined in SSDT. It also drops manual hole patching for reserved PCI/MEM/CPU hoptlug MMIO resources and utilizes the fact that MMIO resources are reserved by respective child /i.e. PHPR, MHPD, PRES/ containers. Signed-off-by: Igor Mammedov imamm...@redhat.com This is a good idea. It's simpler to just do this in the SSDT than to split it between DSDT and SSDT. The AML's purpose is just to build a static value anyway. Paolo I think Marcel has patches going in exactly the reverse direction with this. Marcel? Actually I added the CRS for the extra root buses in SSDT. The reason is that it depends on user input. By the way, I have the code for creating the CRS in code rather that in the file based on Igor's series. Thanks, Marcel
Re: [Qemu-devel] [PATCH v3 2/8] pc: acpi: decribe bridge device as not hotpluggable
On Mon, 19 Jan 2015 14:46:38 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Fri, Dec 19, 2014 at 11:46:58AM +, Igor Mammedov wrote:
when bridge hotplug is disabled, i.e. for machine
types less then 2.0, bridge device was created as
hotpluggable by mistake since commit 133a2da (2.1).
Fix it by just creating it as a present device as
it was done in 1.7.
Signed-off-by: Igor Mammedov imamm...@redhat.com
I still don't get this. Under 1.7 all devices are hotpluggable
unless disabled by device class. Bridges were hotpluggable too.
See 72c194f7e75cb64b2558111cb111adb49fbf4097, function
acpi_get_hotplug_info.
So if the idea is to match 1.7, this does not do it.
Yep, I was wrong about 1.7.
Let's see:
1.7 - all PCI devices created as hotpluggable entries
99fd437d - creates bridge as non hotpluggable unconditionally
133a2da4 - creates bridge as
1 - non hotpluggable if bridge hotplug supported - subtree is creeated
2 - if bridge hotplug isn't supported, it creates bridge as hotpluggable -
subtree is not created
so I guess, I should maintain what 133a2da4 does, drop this patch and amend 8/8
What, then, is the purpose of this patch?
---
hw/i386/acpi-build.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index a4d0c0c..c151fde 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -919,7 +919,7 @@ static void build_pci_bus_end(PCIBus *bus, void
*bus_state)
}
}
-if (!dc-hotpluggable || bridge_in_acpi) {
+if (!dc-hotpluggable || pc-is_bridge) {
clear_bit(slot, slot_hotplug_enable);
}
}
--
1.8.3.1
Re: [Qemu-devel] [PATCH v3 5/8] acpi: move generic aml building helpers into dedictated file
On Mon, 19 Jan 2015 17:08:43 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Fri, Dec 19, 2014 at 11:47:01AM +, Igor Mammedov wrote:
the will be later used for composing AML primitives
and all that could be reused later for ARM machines
as well.
Signed-off-by: Igor Mammedov imamm...@redhat.com
Pls rename acpi-build-utils: using dashes, and
build in name consistent with build in function
names.
Sure.
Do you mean s/acpi_gen_utils/acpi-build-utils/
---
v2:
fix wrong ident in moved code
---
hw/acpi/Makefile.objs| 1 +
hw/acpi/acpi_gen_utils.c | 166
+++
hw/i386/acpi-build.c | 162
+-
include/hw/acpi/acpi_gen_utils.h | 23 ++
4 files changed, 192 insertions(+), 160 deletions(-)
create mode 100644 hw/acpi/acpi_gen_utils.c
create mode 100644 include/hw/acpi/acpi_gen_utils.h
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index acd2389..4237232 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -1,3 +1,4 @@
common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o
common-obj-$(CONFIG_ACPI) += memory_hotplug.o
common-obj-$(CONFIG_ACPI) += acpi_interface.o
+common-obj-$(CONFIG_ACPI) += acpi_gen_utils.o
diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
new file mode 100644
index 000..5f64c37
--- /dev/null
+++ b/hw/acpi/acpi_gen_utils.c
Pls copy header from hw/i386/acpi-build.c:
(we can drop Fabrice's and Kevin's copyright on this file since the specific
functions were all written by myself).
And I guess update the copyright.
/* Support for generating ACPI tables and passing them to Guests
*
* Copyright (C) 2014 Red Hat Inc
*
* Author: Michael S. Tsirkin m...@redhat.com
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
* You should have received a copy of the GNU General Public License along
* with this program; if not, see http://www.gnu.org/licenses/.
*/
@@ -0,0 +1,166 @@
+#include stdio.h
+#include stdarg.h
+#include assert.h
+#include stdbool.h
+#include hw/acpi/acpi_gen_utils.h
+
+GArray *build_alloc_array(void)
+{
+return g_array_new(false, true /* clear */, 1);
+}
+
+void build_free_array(GArray *array)
+{
+g_array_free(array, true);
+}
+
+void build_prepend_byte(GArray *array, uint8_t val)
+{
+g_array_prepend_val(array, val);
+}
+
+void build_append_byte(GArray *array, uint8_t val)
+{
+g_array_append_val(array, val);
+}
+
+void build_append_array(GArray *array, GArray *val)
+{
+g_array_append_vals(array, val-data, val-len);
+}
+
+#define ACPI_NAMESEG_LEN 4
+
+void GCC_FMT_ATTR(2, 3)
+build_append_nameseg(GArray *array, const char *format, ...)
+{
+/* It would be nicer to use g_string_vprintf but it's only there in
2.22 */
+char s[] = ;
+int len;
+va_list args;
+
+va_start(args, format);
+len = vsnprintf(s, sizeof s, format, args);
+va_end(args);
+
+assert(len = ACPI_NAMESEG_LEN);
+
+g_array_append_vals(array, s, len);
+/* Pad up to ACPI_NAMESEG_LEN characters if necessary. */
+g_array_append_vals(array, , ACPI_NAMESEG_LEN - len);
+}
+
+/* 5.4 Definition Block Encoding */
+enum {
+PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
+PACKAGE_LENGTH_2BYTE_SHIFT = 4,
+PACKAGE_LENGTH_3BYTE_SHIFT = 12,
+PACKAGE_LENGTH_4BYTE_SHIFT = 20,
+};
+
+void build_prepend_package_length(GArray *package, unsigned min_bytes)
+{
+uint8_t byte;
+unsigned length = package-len;
+unsigned length_bytes;
+
+if (length + 1 (1 PACKAGE_LENGTH_1BYTE_SHIFT)) {
+length_bytes = 1;
+} else if (length + 2 (1 PACKAGE_LENGTH_3BYTE_SHIFT)) {
+length_bytes = 2;
+} else if (length + 3 (1 PACKAGE_LENGTH_4BYTE_SHIFT)) {
+length_bytes = 3;
+} else {
+length_bytes = 4;
+}
+
+/* Force length to at least min_bytes.
+ * This wastes memory but that's how bios did it.
+ */
+length_bytes = MAX(length_bytes, min_bytes);
+
+/* PkgLength is the length of the inclusive length of the data. */
+length += length_bytes;
+
+switch (length_bytes) {
+case 1:
+byte = length;
+build_prepend_byte(package, byte);
+
[Qemu-devel] target-tricore: Possible bug in get_mtcr()
Coverity[*] points out:
*** CID 1264337: Logically dead code (DEADCODE)
/target-tricore/translate.c: 348 in gen_mtcr()
342 #define E(ADDRESS, REG, FEATURE) A(ADDRESS, REG, FEATURE)
343 static inline void gen_mtcr(CPUTriCoreState *env, DisasContext
*ctx, TCGv r1,
344 int32_t offset)
345 {
346 if (ctx-hflags TRICORE_HFLAG_SM) {
347 /* since we're caching PSW make this a special case */
CID 1264337: Logically dead code (DEADCODE)
Execution cannot reach this statement: if (offset == 65028) {
ge
348 if (offset == 0xfe04) {
349 gen_helper_psw_write(cpu_env, r1);
350 } else {
351 switch (offset) {
352 #include csfr.def
353 }
Correct, because TRICORE_HFLAG_SM is zero:
#define TRICORE_HFLAG_UM0 0x2 /* user mode-0 flag */
#define TRICORE_HFLAG_UM1 0x1 /* user mode-1 flag */
#define TRICORE_HFLAG_SM 0x0 /* kernel mode flag */
Shouls this perhaps be (ctx-hflags (1 TRICORE_HFLAG_SM))?
[*] https://scan.coverity.com/projects/378
Re: [Qemu-devel] [RFC 14/47] acpi: add acpi_notify() term
On Mon, 19 Jan 2015 13:32:48 +0100 Paolo Bonzini pbonz...@redhat.com wrote: On 19/12/2014 03:02, Igor Mammedov wrote: +AcpiAml var = aml_allocate_internal(0, NON_BLOCK); +build_append_byte(var.buf, 0x86); /* NotifyOp */ \ Extra backslash. Thanks, I'll fix it. Paolo +aml_append(var, arg1);
[Qemu-devel] [PATCH] qemu-ga-win: Document 'guest-set-time' limitation
The command implementation for Windows guest has this limitation. If no time to set has been provided the documentation for the command states that time should be read from RTC. However, on Windows bare GetSystemTime() is used, which does not read anything from RTC rather than return system time. Yeah, that system time which is wrong (after stop cont) and which we want to set. However, there's no simple way to read RTC on windows yet [1], so until the time somebody comes with bright implementation, we should at least document the command implementation limitation. 1: http://msdn.microsoft.com/en-us/library/aa908981.aspx Signed-off-by: Michal Privoznik mpriv...@redhat.com --- qga/qapi-schema.json | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json index 376e79f..91821ef 100644 --- a/qga/qapi-schema.json +++ b/qga/qapi-schema.json @@ -121,7 +121,10 @@ # given value, then sets the Hardware Clock (RTC) to the # current System Time. This will make it easier for a guest # to resynchronize without waiting for NTP. If no @time is -# specified, then the time to set is read from RTC. +# specified, then the time to set is read from RTC. On Windows +# guests there's implementation limitation that does not read the +# time from RTC if no time has been provided. Users are advised +# to allways pass a value for Windows guests. # # @time: #optional time of nanoseconds, relative to the Epoch #of 1970-01-01 in UTC. -- 2.0.5
Re: [Qemu-devel] [PATCH v3 5/8] acpi: move generic aml building helpers into dedictated file
On Tue, Jan 20, 2015 at 10:24:41AM +0100, Igor Mammedov wrote:
On Mon, 19 Jan 2015 17:08:43 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Fri, Dec 19, 2014 at 11:47:01AM +, Igor Mammedov wrote:
the will be later used for composing AML primitives
and all that could be reused later for ARM machines
as well.
Signed-off-by: Igor Mammedov imamm...@redhat.com
Pls rename acpi-build-utils: using dashes, and
build in name consistent with build in function
names.
Sure.
Do you mean s/acpi_gen_utils/acpi-build-utils/
Exactly. I'd do so myself, but it's best to keep the copyrights
straight from the beginning, and you have a bunch of follow-up
patches as well.
---
v2:
fix wrong ident in moved code
---
hw/acpi/Makefile.objs| 1 +
hw/acpi/acpi_gen_utils.c | 166
+++
hw/i386/acpi-build.c | 162
+-
include/hw/acpi/acpi_gen_utils.h | 23 ++
4 files changed, 192 insertions(+), 160 deletions(-)
create mode 100644 hw/acpi/acpi_gen_utils.c
create mode 100644 include/hw/acpi/acpi_gen_utils.h
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index acd2389..4237232 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -1,3 +1,4 @@
common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o pcihp.o cpu_hotplug.o
common-obj-$(CONFIG_ACPI) += memory_hotplug.o
common-obj-$(CONFIG_ACPI) += acpi_interface.o
+common-obj-$(CONFIG_ACPI) += acpi_gen_utils.o
diff --git a/hw/acpi/acpi_gen_utils.c b/hw/acpi/acpi_gen_utils.c
new file mode 100644
index 000..5f64c37
--- /dev/null
+++ b/hw/acpi/acpi_gen_utils.c
Pls copy header from hw/i386/acpi-build.c:
(we can drop Fabrice's and Kevin's copyright on this file since the specific
functions were all written by myself).
And I guess update the copyright.
/* Support for generating ACPI tables and passing them to Guests
*
* Copyright (C) 2014 Red Hat Inc
*
* Author: Michael S. Tsirkin m...@redhat.com
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
* You should have received a copy of the GNU General Public License along
* with this program; if not, see http://www.gnu.org/licenses/.
*/
@@ -0,0 +1,166 @@
+#include stdio.h
+#include stdarg.h
+#include assert.h
+#include stdbool.h
+#include hw/acpi/acpi_gen_utils.h
+
+GArray *build_alloc_array(void)
+{
+return g_array_new(false, true /* clear */, 1);
+}
+
+void build_free_array(GArray *array)
+{
+g_array_free(array, true);
+}
+
+void build_prepend_byte(GArray *array, uint8_t val)
+{
+g_array_prepend_val(array, val);
+}
+
+void build_append_byte(GArray *array, uint8_t val)
+{
+g_array_append_val(array, val);
+}
+
+void build_append_array(GArray *array, GArray *val)
+{
+g_array_append_vals(array, val-data, val-len);
+}
+
+#define ACPI_NAMESEG_LEN 4
+
+void GCC_FMT_ATTR(2, 3)
+build_append_nameseg(GArray *array, const char *format, ...)
+{
+/* It would be nicer to use g_string_vprintf but it's only there in
2.22 */
+char s[] = ;
+int len;
+va_list args;
+
+va_start(args, format);
+len = vsnprintf(s, sizeof s, format, args);
+va_end(args);
+
+assert(len = ACPI_NAMESEG_LEN);
+
+g_array_append_vals(array, s, len);
+/* Pad up to ACPI_NAMESEG_LEN characters if necessary. */
+g_array_append_vals(array, , ACPI_NAMESEG_LEN - len);
+}
+
+/* 5.4 Definition Block Encoding */
+enum {
+PACKAGE_LENGTH_1BYTE_SHIFT = 6, /* Up to 63 - use extra 2 bits. */
+PACKAGE_LENGTH_2BYTE_SHIFT = 4,
+PACKAGE_LENGTH_3BYTE_SHIFT = 12,
+PACKAGE_LENGTH_4BYTE_SHIFT = 20,
+};
+
+void build_prepend_package_length(GArray *package, unsigned min_bytes)
+{
+uint8_t byte;
+unsigned length = package-len;
+unsigned length_bytes;
+
+if (length + 1 (1 PACKAGE_LENGTH_1BYTE_SHIFT)) {
+length_bytes = 1;
+} else if (length + 2 (1 PACKAGE_LENGTH_3BYTE_SHIFT)) {
+length_bytes = 2;
+} else if (length + 3 (1 PACKAGE_LENGTH_4BYTE_SHIFT)) {
+length_bytes = 3;
+} else {
+length_bytes = 4;
+}
+
+/* Force length to at least min_bytes.
+
Re: [Qemu-devel] [PATCH v3 8/8] pc: acpi-build: simplify PCI bus tree generation
On Mon, 19 Jan 2015 17:23:41 +0200
Michael S. Tsirkin m...@redhat.com wrote:
On Fri, Dec 19, 2014 at 11:47:04AM +, Igor Mammedov wrote:
it basicaly does the same as original approach,
* just without bus/notify tables tracking (less obscure)
which is easier to follow.
* drops unnecessary loops and bitmaps,
creating devices and notification method in the same loop.
* saves us ~100LOC
change in behavior:
* generate hotpluggable device entries for empty slots
only if BUS itself is hotpluggable, otherwise do not
create them.
Hmm so how do you create a machine where this applies?
Can you clarify?
it only can happen if bridge device marked as non hotpluggable,
i.e. not happens in practice for now. Perhaps I should drop
this comment altogether.
Signed-off-by: Igor Mammedov imamm...@redhat.com
Overall looks fine to me, I'm fine with appying this after rebase.
Although I would prefer it if we didn't add so many scope
operations: I think it's cleaner to add content to devices
directly, but not a must.
scope here is used to avoid construction of pcinohp template manually
by hand. and reuse current pcihohp template. However it should be easy
to drop scopes in following conversion where devices are constructed
manually. I'll take care of it in that series during its rebasing.
---
v3:
* use hotpluggable device object instead of not hotpluggable
for non present devices, and add it only when bus itself is
hotpluggable
---
hw/i386/acpi-build.c | 265
+++
1 file changed, 79 insertions(+), 186 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 94202b5..a893f5e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -103,7 +103,6 @@ typedef struct AcpiPmInfo {
typedef struct AcpiMiscInfo {
bool has_hpet;
bool has_tpm;
-DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
const unsigned char *dsdt_code;
unsigned dsdt_size;
uint16_t pvpanic_port;
@@ -647,69 +646,37 @@ static void acpi_set_pci_info(void)
}
}
-static void build_pci_bus_state_init(AcpiBuildPciBusHotplugState *state,
- AcpiBuildPciBusHotplugState *parent,
- bool pcihp_bridge_en)
+static void build_append_pcihp_notify_entry(GArray *method, int slot)
{
-state-parent = parent;
-state-device_table = build_alloc_array();
-state-notify_table = build_alloc_array();
-state-pcihp_bridge_en = pcihp_bridge_en;
-}
-
-static void build_pci_bus_state_cleanup(AcpiBuildPciBusHotplugState *state)
-{
-build_free_array(state-device_table);
-build_free_array(state-notify_table);
-}
+GArray *ifctx;
-static void *build_pci_bus_begin(PCIBus *bus, void *parent_state)
-{
-AcpiBuildPciBusHotplugState *parent = parent_state;
-AcpiBuildPciBusHotplugState *child = g_malloc(sizeof *child);
-
-build_pci_bus_state_init(child, parent, parent-pcihp_bridge_en);
+ifctx = build_alloc_array();
+build_append_byte(ifctx, 0x7B); /* AndOp */
+build_append_byte(ifctx, 0x68); /* Arg0Op */
+build_append_int(ifctx, 0x1U slot);
+build_append_byte(ifctx, 0x00); /* NullName */
+build_append_byte(ifctx, 0x86); /* NotifyOp */
+build_append_namestring(ifctx, S%.02X, PCI_DEVFN(slot, 0));
+build_append_byte(ifctx, 0x69); /* Arg1Op */
-return child;
+/* Pack it up */
+build_package(ifctx, 0xA0 /* IfOp */);
+build_append_array(method, ifctx);
+build_free_array(ifctx);
}
-static void build_pci_bus_end(PCIBus *bus, void *bus_state)
+static void build_append_pci_bus_devices(GArray *parent_scope, PCIBus *bus,
+ bool pcihp_bridge_en)
{
-AcpiBuildPciBusHotplugState *child = bus_state;
-AcpiBuildPciBusHotplugState *parent = child-parent;
GArray *bus_table = build_alloc_array();
-DECLARE_BITMAP(slot_hotplug_enable, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_present, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_system, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_vga, PCI_SLOT_MAX);
-DECLARE_BITMAP(slot_device_qxl, PCI_SLOT_MAX);
-uint8_t op;
-int i;
+GArray *method = NULL;
QObject *bsel;
-GArray *method;
-bool bus_hotplug_support = false;
-
-/*
- * Skip bridge subtree creation if bridge hotplug is disabled
- * to make acpi tables compatible with legacy machine types.
- */
-if (!child-pcihp_bridge_en bus-parent_dev) {
-return;
-}
+PCIBus *sec;
+int i;
if (bus-parent_dev) {
-op = 0x82; /* DeviceOp */
-build_append_namestring(bus_table, S%.02X,
- bus-parent_dev-devfn);
-
Re: [Qemu-devel] [v6][PATCH 01/10] i440fx: make types configurable at run-time
On Di, 2015-01-20 at 10:48 +0800, Chen, Tiejun wrote: On 2015/1/19 19:36, Gerd Hoffmann wrote: On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote: From: Michael S. Tsirkin m...@redhat.com Xen wants to supply a different pci and host devices, inheriting i440fx devices. Make types configurable. Description is misleading, this isn't about xen but about IGD Its about IGD passthrough in Xen side. As far I can see the only really xen specific stuff here is the pci pass-through bits, i.e. how we handle the IGD device itself. The northbridge isa-bridge emulation extensions needed for IGD should be pretty much common for IGD passthough on xen, IGD passthrough on kvm (vfio) and IGD virtualization (xengt+kvmgt). cheers, Gerd
Re: [Qemu-devel] [PATCH 2/3 V3] s390: implement pci instructions
This patch makes Coverity unhappy:
*** CID 1264326: Unintended sign extension (SIGN_EXTENSION)
/hw/s390x/s390-pci-inst.c: 787 in stpcifc_service_call()
781 stq_p(fib.pal, pbdev-pal);
782 stq_p(fib.iota, pbdev-g_iota);
783 stq_p(fib.aibv, pbdev-routes.adapter.ind_addr);
784 stq_p(fib.aisb, pbdev-routes.adapter.summary_addr);
785 stq_p(fib.fmb_addr, pbdev-fmb_addr);
786
CID 1264326: Unintended sign extension (SIGN_EXTENSION)
Suspicious implicit sign extension: pbdev-isc with type
unsigned char (8 bits, unsigned) is promoted in (pbdev-isc
28) | (pbdev-noi 16) to type int (32 bits, signed), then
sign-extended to type unsigned long (64 bits, unsigned). If
(pbdev-isc 28) | (pbdev-noi 16) is greater than
0x7FFF, the upper bits of the result will all be 1.
787 data = (pbdev-isc 28) | (pbdev-noi 16) |
788(pbdev-routes.adapter.ind_offset 8) | (pbdev-sum 7) |
789pbdev-routes.adapter.summary_offset;
790 stw_p(fib.data, data);
791
792 if (pbdev-fh ENABLE_BIT_OFFSET) {
*** CID 1264324: Unintended sign extension (SIGN_EXTENSION)
/hw/s390x/s390-pci-inst.c: 787 in stpcifc_service_call()
781 stq_p(fib.pal, pbdev-pal);
782 stq_p(fib.iota, pbdev-g_iota);
783 stq_p(fib.aibv, pbdev-routes.adapter.ind_addr);
784 stq_p(fib.aisb, pbdev-routes.adapter.summary_addr);
785 stq_p(fib.fmb_addr, pbdev-fmb_addr);
786
CID 1264324: Unintended sign extension (SIGN_EXTENSION)
Suspicious implicit sign extension: pbdev-noi with type
unsigned short (16 bits, unsigned) is promoted in (pbdev-isc
28) | (pbdev-noi 16) to type int (32 bits, signed), then
sign-extended to type unsigned long (64 bits, unsigned). If
(pbdev-isc 28) | (pbdev-noi 16) is greater than
0x7FFF, the upper bits of the result will all be 1.
787 data = (pbdev-isc 28) | (pbdev-noi 16) |
788(pbdev-routes.adapter.ind_offset 8) | (pbdev-sum 7) |
789pbdev-routes.adapter.summary_offset;
790 stw_p(fib.data, data);
791
792 if (pbdev-fh ENABLE_BIT_OFFSET) {
Does this code work as intended?
Re: [Qemu-devel] [PATCH] s390: Plug memory leak on s390_pci_generate_event() error path
On 20/01/2015 10:56, Markus Armbruster wrote:
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/s390x/s390-pci-bus.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 1201b8d..d25ac74 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -187,7 +187,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh,
uint32_t fid, uint64_t faddr, uint32_t e)
{
-SeiContainer *sei_cont = g_malloc0(sizeof(SeiContainer));
+SeiContainer *sei_cont;
S390pciState *s = S390_PCI_HOST_BRIDGE(
object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
@@ -195,6 +195,7 @@ static void s390_pci_generate_event(uint8_t cc, uint16_t
pec, uint32_t fh,
return;
}
+sei_cont = g_malloc0(sizeof(SeiContainer));
sei_cont-fh = fh;
sei_cont-fid = fid;
sei_cont-cc = cc;
A patch for this had been sent already, though I prefer yours.
Paolo
Re: [Qemu-devel] [PATCH RFC v6 08/20] dataplane: allow virtio-1 devices
On Tue, 20 Jan 2015 10:43:31 + Stefan Hajnoczi stefa...@gmail.com wrote: On Thu, Dec 11, 2014 at 02:25:10PM +0100, Cornelia Huck wrote: Handle endianness conversion for virtio-1 virtqueues correctly. Note that dataplane now needs to be built per-target. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/block/dataplane/virtio-blk.c |4 +- hw/scsi/virtio-scsi-dataplane.c |2 +- hw/virtio/Makefile.objs |2 +- hw/virtio/dataplane/Makefile.objs |2 +- hw/virtio/dataplane/vring.c | 86 ++--- include/hw/virtio/dataplane/vring-accessors.h | 75 + include/hw/virtio/dataplane/vring.h | 14 +--- 7 files changed, 131 insertions(+), 54 deletions(-) create mode 100644 include/hw/virtio/dataplane/vring-accessors.h This patch is independent of VIRTIO 1.0 and can be merged separately (faster). Yep, I've pulled it in front of my queue. diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 1222a37..2d8cc15 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -16,7 +16,9 @@ #include qemu/iov.h #include qemu/thread.h #include qemu/error-report.h +#include hw/virtio/virtio-access.h #include hw/virtio/dataplane/vring.h +#include hw/virtio/dataplane/vring-accessors.h I like your vring-accessors.h approach better than the inline virtio_ld/st_p() in my patch. Nice. @@ -154,15 +157,18 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) } -static int get_desc(Vring *vring, VirtQueueElement *elem, +static int get_desc(VirtIODevice *vdev, Vring *vring, VirtQueueElement *elem, struct vring_desc *desc) Since we copy in struct vring_desc anyway, it's cleaner to byteswap the fields once instead of remembering to do it each time we need to access a field. The copy_in_vring_desc() function is one thing I prefer I about my patch. Agreed, that makes the code cleaner. I've prepared a version of this patch using copy_in_vring_desc() and I'll post it when it survives some light testing on my side.
Re: [Qemu-devel] [PATCH 2/3 V3] s390: implement pci instructions
Markus Armbruster arm...@redhat.com writes:
Cornelia Huck cornelia.h...@de.ibm.com writes:
On Tue, 20 Jan 2015 10:45:41 +0100
Markus Armbruster arm...@redhat.com wrote:
This patch makes Coverity unhappy:
*** CID 1264326: Unintended sign extension (SIGN_EXTENSION)
/hw/s390x/s390-pci-inst.c: 787 in stpcifc_service_call()
781 stq_p(fib.pal, pbdev-pal);
782 stq_p(fib.iota, pbdev-g_iota);
783 stq_p(fib.aibv, pbdev-routes.adapter.ind_addr);
784 stq_p(fib.aisb, pbdev-routes.adapter.summary_addr);
785 stq_p(fib.fmb_addr, pbdev-fmb_addr);
786
CID 1264326: Unintended sign extension (SIGN_EXTENSION)
Suspicious implicit sign extension: pbdev-isc with type
unsigned char (8 bits, unsigned) is promoted in (pbdev-isc
28) | (pbdev-noi 16) to type int (32 bits, signed), then
sign-extended to type unsigned long (64 bits, unsigned). If
(pbdev-isc 28) | (pbdev-noi 16) is greater than
0x7FFF, the upper bits of the result will all be 1.
787 data = (pbdev-isc 28) | (pbdev-noi 16) |
788 (pbdev-routes.adapter.ind_offset 8) | (pbdev-sum 7) |
789pbdev-routes.adapter.summary_offset;
790 stw_p(fib.data, data);
791
792 if (pbdev-fh ENABLE_BIT_OFFSET) {
There's a fix for this (and the memory leak):
http://marc.info/?l=qemu-develm=142124886620078w=2
The patch is sitting in my queue, will send with the next pile of s390x
updates.
I can't see how
@@ -787,7 +787,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1,
uint64_t fiba)
data = (pbdev-isc 28) | (pbdev-noi 16) |
(pbdev-routes.adapter.ind_offset 8) | (pbdev-sum 7) |
pbdev-routes.adapter.summary_offset;
-stw_p(fib.data, data);
+stl_p(fib.data, data);
if (pbdev-fh ENABLE_BIT_OFFSET) {
fib.fc |= 0x80;
fixes the implicit sign extension within the assignment preceding it.
Let me explain it again real slow:
1. pbdev-isc gets promoted from uint8_t to int as operand of binary
(usual arithmetic conversions ISO/IEC 9899:1999 6.3.1.8)
2. The int result is shifted left 28 bits. This can set the MSB.
3. Likewise: pbdev-noi gets promoted from uint64_t to int, and shifted
left 16 bits.
4. The two shift results stay int and get ored.
5. pbdev-routes.adapter.ind_offset stays uint64_t, and is shifted left
8 bits.
6. The next or's left operand is the int result of 4 and the right
operant is the uint64_t result of 5. Therefore, the left operand is
*sign-extended* from int to uint64_t. This copies bit#7 of
pbdev-isc to bits#31..63. Whoops.
I neglected to say: we don't currently use the upper 32 bits, and as
long as we do that, the sign extension is harmless. I'd recommend to
avoid it all the same, for robustness, and to hush up Coverity.
Regarding the leak, I prefer my patch, because it avoids the free on
error. But you're the maintainer.
Re: [Qemu-devel] [PATCH] kvm/apic: fix 2.2-2.1 migration
Paolo Bonzini pbonz...@redhat.com writes:
The wait_for_sipi field is set back to 1 after an INIT, so it was not
effective to reset it in kvm_apic_realize. Introduce a reset callback
and reset wait_for_sipi there.
Reported-by: Igor Mammedov imamm...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/i386/kvm/apic.c | 10 +++---
hw/intc/apic_common.c | 5 +
include/hw/i386/apic_internal.h | 1 +
3 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 271e97f..5b47056 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -171,12 +171,15 @@ static const MemoryRegionOps kvm_apic_io_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static void kvm_apic_realize(DeviceState *dev, Error **errp)
+static void kvm_apic_reset(APICCommonState *s)
{
-APICCommonState *s = APIC_COMMON(dev);
-
/* Not used by KVM, which uses the CPU mp_state instead. */
s-wait_for_sipi = 0;
+}
+
+static void kvm_apic_realize(DeviceState *dev, Error **errp)
+{
+APICCommonState *s = APIC_COMMON(dev);
memory_region_init_io(s-io_memory, NULL, kvm_apic_io_ops, s,
kvm-apic-msi,
APIC_SPACE_SIZE);
@@ -191,6 +194,7 @@ static void kvm_apic_class_init(ObjectClass *klass, void
*data)
APICCommonClass *k = APIC_COMMON_CLASS(klass);
k-realize = kvm_apic_realize;
+k-reset = kvm_apic_reset;
k-set_base = kvm_apic_set_base;
k-set_tpr = kvm_apic_set_tpr;
k-get_tpr = kvm_apic_get_tpr;
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 4e62f25..d9bb188 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -178,6 +178,7 @@ bool apic_next_timer(APICCommonState *s, int64_t
current_time)
void apic_init_reset(DeviceState *dev)
{
APICCommonState *s = APIC_COMMON(dev);
+APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
int i;
if (!s) {
*** CID 1264327: Dereference before null check (REVERSE_INULL)
/hw/intc/apic_common.c: 184 in apic_init_reset()
178 void apic_init_reset(DeviceState *dev)
179 {
180 APICCommonState *s = APIC_COMMON(dev);
181 APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
182 int i;
183
CID 1264327: Dereference before null check (REVERSE_INULL)
Null-checking s suggests that it may be null, but it has
already been dereferenced on all paths leading to the check.
184 if (!s) {
185 return;
186 }
187 s-tpr = 0;
188 s-spurious_vec = 0xff;
189 s-log_dest = 0;
[...]
Re: [Qemu-devel] [PATCH v2 1/4] pc: append ssdt-misc.dsl to the DSDT
On Mon, 19 Jan 2015 21:29:57 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Jan 19, 2015 at 06:26:55PM +0100, Paolo Bonzini wrote: On 19/01/2015 18:14, Igor Mammedov wrote: I'm fine with moving SMC out of the per-machine-type AML, should be a separate patch anyway. But patch-able SMC being in DSDT is our mistake that we allowed it to slip there and should be better moved to SSDT rather than staying in DSDT and making thing more complex. It's also candidate for trimming, i.e. dropping it from tables altogether if device is not present in QEMU, same applies to _S[34] Packages when respective features are disabled and to PEVT device template. Yes, trimming is better than putting it in the DSDT, at least for simple devices such as SMC and pvpanic. So are we dropping 1-2/4 from this series? I need to know on top of what to rebase. I'll take care of moving SMC to SSDT. simpler. However, it also complicates backwards compatibility, so merge it with the DSDT. What are these complications? The complication arises if we want to make the SSDT exactly the same for all QEMU versions, given a (machine type, command line) pair. Then you either cannot do any change to ssdt-misc, or you have to keep different copies for each machine type. With resizable ROM blobs in master, there shouldn't be an issue with migration in new QEMU versions if size of SSDT changes. There is only a very small issue that remains (the RSDP pointer is wrong if the size changes), Yes - for new machine types I'll send a patch to put it in memory. For old ones - there's a race, and it's painful to fix. If we do want to try fixing it, one solution is to fail migration if attempted before rsdp is shadowed. Useful? There were my patches on list that move RSDT at the start of blob, which fixes issue for new machine types. That patches however weren't doing good job for old machine types. I can respin that series fixing new machines and we can fix old machines in separate patch later. so we probably should apply anyway the patch of mine that allows the DSDT size to change; and we probably should pay attention to SSDT, and version it. (Let's just ignore the SSDT was exactly what I feared when I disagreed with putting in resizable ROM blobs first. But now that it's in, I cannot really argue otherwise). I don't have a strong opinion here. you guys arrive at a rough consensus.:w So question is if we still need SSDT version-ing and per machine type SSDT compatibility? /it's better not to do version-ing at all if it could be avoided, due to maintenance headache it brings along/ I'm okay with re-evaluating that after your patches go in. Paolo
Re: [Qemu-devel] [PATCH 0/2] qcow2: Add two more unalignment checks
Am 19.01.2015 um 22:09 hat Max Reitz geschrieben: On 2015-01-19 at 16:04, Eric Blake wrote: On 01/19/2015 01:49 PM, Max Reitz wrote: With the series adding unalignment checks and the series reworking the zero cluster expansion code overlapping, the unalignment checks have not been implemented in the latter code. This series fixes it. There are other places which would require unalignment checks, like the offsets of L1 tables, especially for snapshots; but because it would be best to add these checks in the function which reads the snapshot table, this would make images with broken snapshots completely unusable, which is something I opted to avoid for now. That's something I noticed, too: For qemu-img check, our qcow2_open() checks may be to strict. With a corrupted snapshot table, it won't even run. Perhaps we should be laxer with some checks with BDRV_O_CHECK, and for example just set s-nb_snapshots = 0 if loading the table fails. Ideally, we need to make the qcow2 repair function repair such cases, but until that is done there is not much we can do about them. What's the best repair? That's what I was asking myself... Read the data from the unaligned location, and write a fresh copy into a new aligned allocation? Maybe. Maybe there is no way of repairing them and the only way is asking the user whether it's fine to delete the snapshot (qemu-img check -r make_consistent_whatever_it_takes). Also, the question remains for every unaligned data structure: L2 tables, data clusters… The refcount structures are the only ones that can be easily recovered. For data clusters, reading from the unaligned location would probably be best; for L2 tables… maybe the same, then run a consistency check on the data and if it seems off™, throw the whole L2 table away. Reading from the unaligned location doesn't help. I have never seen an image whose L2 entries contained valid entries, except for the least significant few bits. If your table is corrupted, it's usually garbage from start to end. I think the only sane option is to drop the entries. The big question is what should be used to replace them. Kevin
Re: [Qemu-devel] [PATCH 0/5] target-arm: ARM64: Adding EL1 AARCH32 guest support
On 20.01.2015 01:30, Greg Bellows wrote: Added support for running an AArch32 guest on a AArch64 KVM host. Support has only been added to the QEMU machvirt machine. The addition of CPU properties specifiable from the command line were added to allow disablement of AArch64 execution state hereby forcing EL1 to be AArch32. The new CPU command line propoerty is -aarch64 that is specified as follows: aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-a57,-aarch64 ... Hi! It seems a little confusing for me to specify '-aarch64' when forcing AArch32 execution state. Why don't just specify 'aarch32' in command line instead of '-aarch64' construction? BTW, /propoerty/property/ Best regards, Serge Support also added to support uncompressed images (Image) for aarch32. Greg Bellows (5): target-arm: Add ARM CPU feature parsing target-arm: Add feature parsing to virt target-arm: Add 32/64-bit register sync target-arm: Add AArch32 guest support to KVM64 target-arm: Adjust kernel load address for Image hw/arm/boot.c | 33 +- hw/arm/virt.c | 21 ++-- target-arm/cpu.c| 45 - target-arm/helper-a64.c | 5 +-- target-arm/internals.h | 89 + target-arm/kvm64.c | 21 +--- target-arm/op_helper.c | 6 ++-- 7 files changed, 204 insertions(+), 16 deletions(-)
[Qemu-devel] [Bug 1323001] Re: Netlink socket support for MIPS*
** Changed in: qemu
Status: Fix Committed = Fix Released
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1323001
Title:
Netlink socket support for MIPS*
Status in QEMU:
Fix Released
Bug description:
It seems QEMU does not support Netlink socket support on MIPS*
Trying to compile and run this simple program:
#include libaudit.h
#include stdio.h
#include errno.h
int main()
{
int audit_fd = audit_open ();
printf(fd is %d\n, audit_fd);
printf(errno is %d\n, errno);
return 0;
}
I receive the following output:
$ ./test
fd is -1
errno is 97
$
errno 97 is #define EAFNOSUPPORT97 /* Address family not
supported by protocol */
To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1323001/+subscriptions
Re: [Qemu-devel] [PATCH v3 0/5] Migration Deciphering aid
On (Fri) 26 Dec 2014 [15:42:43], Alexander Graf wrote: Migration is a black hole to most people. One of the biggest reasons for this is that its protocol is a secret, undocumented sauce of code rolling around random parts of the QEMU code base. But what if we simply exposed the description of how the format looks like alongside the actual migration stream? This is what this patch set does. It adds a new section that comes after the end of stream marker (so that it doesn't slow down migration) that contains a JSON description of the device state description. Along with this patch set also comes a python script that can read said JSON from a migration dump and decipher the device state and ram contents of the migration dump using it. With this, you can now fully examine all glorious details that go over the wire when virtual machine state gets dumped, such as during live migration. We discussed the approach taken here during KVM Forum 2013. Originally, my idea was to include a special device that contains the JSON data which can be enabled on demand. Anthony suggested however to just always include the description data after the end marker which I think is a great idea. Example decoded migration: http://csgraf.de/mig/mig.txt Example migration description: http://csgraf.de/mig/mig.desc.txt Presentation: https://www.youtube.com/watch?v=iq1x40Qsrew Slides: https://www.dropbox.com/s/otp2pk2n3g087zp/Live%20Migration.pdf Nice to finally see this! I guess you have a v4 coming soon? Amit
Re: [Qemu-devel] [PATCH v2 1/4] pc: append ssdt-misc.dsl to the DSDT
On Tue, Jan 20, 2015 at 10:59:43AM +0100, Igor Mammedov wrote: On Mon, 19 Jan 2015 21:29:57 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Jan 19, 2015 at 06:26:55PM +0100, Paolo Bonzini wrote: On 19/01/2015 18:14, Igor Mammedov wrote: I'm fine with moving SMC out of the per-machine-type AML, should be a separate patch anyway. But patch-able SMC being in DSDT is our mistake that we allowed it to slip there and should be better moved to SSDT rather than staying in DSDT and making thing more complex. It's also candidate for trimming, i.e. dropping it from tables altogether if device is not present in QEMU, same applies to _S[34] Packages when respective features are disabled and to PEVT device template. Yes, trimming is better than putting it in the DSDT, at least for simple devices such as SMC and pvpanic. So are we dropping 1-2/4 from this series? I need to know on top of what to rebase. I'll take care of moving SMC to SSDT. simpler. However, it also complicates backwards compatibility, so merge it with the DSDT. What are these complications? The complication arises if we want to make the SSDT exactly the same for all QEMU versions, given a (machine type, command line) pair. Then you either cannot do any change to ssdt-misc, or you have to keep different copies for each machine type. With resizable ROM blobs in master, there shouldn't be an issue with migration in new QEMU versions if size of SSDT changes. There is only a very small issue that remains (the RSDP pointer is wrong if the size changes), Yes - for new machine types I'll send a patch to put it in memory. For old ones - there's a race, and it's painful to fix. If we do want to try fixing it, one solution is to fail migration if attempted before rsdp is shadowed. Useful? There were my patches on list that move RSDT at the start of blob, which fixes issue for new machine types. I don't see the point - IMO for new machine types, we can just put RSDP in a memory region, have it migrated. That patches however weren't doing good job for old machine types. I can respin that series fixing new machines and we can fix old machines in separate patch later. I don't think it's worth it since I don't see an easy way for old machine types. A harder way would be to allow rom files to share an MR. We could then stick RSDP at the tail of the MR, and look for it on incoming migration: if there, fix it up. Needs reworking of rom_add_blob API. so we probably should apply anyway the patch of mine that allows the DSDT size to change; and we probably should pay attention to SSDT, and version it. (Let's just ignore the SSDT was exactly what I feared when I disagreed with putting in resizable ROM blobs first. But now that it's in, I cannot really argue otherwise). I don't have a strong opinion here. you guys arrive at a rough consensus.:w So question is if we still need SSDT version-ing and per machine type SSDT compatibility? /it's better not to do version-ing at all if it could be avoided, due to maintenance headache it brings along/ I'm okay with re-evaluating that after your patches go in. Paolo
Re: [Qemu-devel] [PATCH 1/7] isa: add memory space parameter to isa_bus_new
On 19/01/2015 22:28, Hervé Poussineau wrote:
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index f0a3201..8f932c9 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -208,7 +208,7 @@ static void pc_init1(MachineState *machine,
} else {
pci_bus = NULL;
i440fx_state = NULL;
-isa_bus = isa_bus_new(NULL, system_io);
+isa_bus = isa_bus_new(NULL, get_system_memory(), system_io);
no_hpet = 1;
}
isa_bus_irqs(isa_bus, gsi);
I suspect the right thing to do would be the PCI address space, since
the ISA bridge on real hardware has BusMaster+ set in lspci. But if
firmware doesn't set it for virtual machines, passing
get_system_memory() is probably a good idea.
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Re: [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions
Hi,
+static GLchar texture_blit_vert_src[] =
+\n
+#version 300 es\n
+\n
+in vec2 in_position;\n
+in vec2 in_tex_coord;\n
You could calculate the texture coordinate from the position in the
shader, but this is mostly my premature optimization instinct kicking in
instead of a real performance difference considering how few vertices
there are in this case.
Still makes sense. Done.
+out vec2 ex_tex_coord;\n
+\n
+void main(void) {\n
+gl_Position = vec4(in_position.x, in_position.y, 0.0, 1.0);\n
vec4(in_position, 0.0, 1.0) *cough*
Done.
This looks like a list for GL_TRIANGLES instead of GL_TRIANGLE_STRIP.
For GL_TRIANGLE_STRIP, you first define one whole triangle (by
specifying each of the three vertices) and after that, each vertex
results in a new triangle drawn (whose two other vertices are the two
vertices preceding the last one).
Thanks for the nice description.
So the trick to get it done with only four vertexes is to put the
correct points (which are going to be shared by both triangles) into the
middle. I've played around with it a bit without success (and before my
new opengl book arrived), and this 6-point version worked ...
+glUseProgram(gls-texture_blit_prog);
+l_position = glGetAttribLocation(gls-texture_blit_prog,
in_position);
+l_tex_coord = glGetAttribLocation(gls-texture_blit_prog,
in_tex_coord);
+glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0,
in_position);
+glVertexAttribPointer(l_tex_coord, 2, GL_FLOAT, GL_FALSE, 0,
in_tex_coord);
I think it is disallowed to refer to non-OpenGL buffers here in the core
profile. The 4.4 core specification states (section 10.3, vertex
arrays): Vertex data are placed into arrays that are stored in the
server’s address space; the 4.4 compatibility specification states:
Vertex data may also be placed into arrays that are stored in the
client’s address space (described here) or in the server’s address space.
Got this from gles sample code, so that should be ok ;)
I've also seen code explicitly storing vertex data in a buffer, which I
assume is more efficient, but I decided to not care for now, especially
given the small number of vertexes we are processing here.
+return gl_create_link_program(vert_shader, frag_shader);
Minor thing: You are free to delete the shaders after they have been
linked into the program (because the references will be lost when this
function returns).
Done.
+switch (surface-format) {
+case PIXMAN_BE_b8g8r8x8:
+case PIXMAN_BE_b8g8r8a8:
+surface-glformat = GL_BGRA_EXT;
If you want to avoid the _EXT, you could use GL_RGBA here and
texture().bgra in the fragment shader. However, this would require a
different shader for the 32 bit and the 16 bit formats (because the 16
bit format has the right order, apparently).
At least it worked in testing.
I haven't been able to really test 16 bit mode (forcing 16 bit mode in
hw/display/vga.c doesn't count...), so I'm trusting you this works.
-kernel /boot/vmlinux-$whatever -append vga=0x314
Makes the kernel run vesafb with 800x600 @ 16bpp, and thanks to the
little friendly penguin in the upper left corner (CONFIG_LOGO=y) you can
easily spot colors being messed up.
+glPixelStorei(GL_UNPACK_ROW_LENGTH_EXT,
+ surface_stride(surface) /
surface_bytes_per_pixel(surface));
Maybe we should assert that surface_stride(surface) %
surface_bytes_per_pixel(surface) == 0 here.
Done.
thanks,
Gerd
Re: [Qemu-devel] [PATCH RFC v6 10/20] s390x/virtio-ccw: add virtio set-revision call
On Thu, Dec 11, 2014 at 02:25:12PM +0100, Cornelia Huck wrote: From: Thomas Huth th...@linux.vnet.ibm.com Handle the virtio-ccw revision according to what the guest sets. When revision 1 is selected, we have a virtio-1 standard device with byteswapping for the virtio rings. When a channel gets disabled, we have to revert to the legacy behavior in case the next user of the device does not negotiate the revision 1 anymore (e.g. the boot firmware uses revision 1, but the operating system only uses the legacy mode). Note that revisions 0 are still disabled. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/virtio-ccw.c | 52 + hw/s390x/virtio-ccw.h |8 2 files changed, 60 insertions(+) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgpx4xPwEBHXh.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH RFC v6 11/20] s390x/virtio-ccw: support virtio-1 set_vq format
On Thu, Dec 11, 2014 at 02:25:13PM +0100, Cornelia Huck wrote: Support the new CCW_CMD_SET_VQ format for virtio-1 devices. While we're at it, refactor the code a bit and enforce big endian fields (which had always been required, even for legacy). Reviewed-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/virtio-ccw.c | 114 ++--- 1 file changed, 80 insertions(+), 34 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgpUM34wkvCQz.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] target-mips: Clean up switch fall through after commit fecd264
On 20 January 2015 at 09:59, Markus Armbruster arm...@redhat.com wrote: Commit fecd264 added a number of fall-throughs, but neglected to properly document them as intentional. Commit d922445 cleaned that up for many, but not all cases. Take care of the remaining ones. Spotted by Coverity. Signed-off-by: Markus Armbruster arm...@redhat.com --- target-mips/translate.c | 4 1 file changed, 4 insertions(+) diff --git a/target-mips/translate.c b/target-mips/translate.c index e9d86b2..8abc12b 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -18729,6 +18729,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) case OPC_SWL: case OPC_SWR: check_insn_opc_removed(ctx, ISA_MIPS32R6); + /* fall through */ Indent here seems to be out by one? The others look OK. -- PMM
[Qemu-devel] [PULL 0/4] Xen tree 2015-01-20
The following changes since commit 74acb99737dbedd86654d660c0c20815139a873c: Merge remote-tracking branch 'remotes/kraxel/tags/pull-console-20150119-1' into staging (2015-01-19 13:37:05 +) are available in the git repository at: git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-2015-01-20 for you to fetch changes up to 085bde8e8f9bd4fb06e010810991b26aba795fb2: xen: add a lock for the mapcache (2015-01-20 11:09:54 +) Paolo Bonzini (2): xen: do not use __-named variables in mapcache xen: add a lock for the mapcache Paul Durrant (2): Add device listener interface Xen: Use the ioreq-server API when available configure | 29 ++ hw/core/qdev.c | 53 ++ include/hw/qdev-core.h | 10 ++ include/hw/xen/xen_common.h | 223 +++ include/qemu/typedefs.h |1 + trace-events|9 ++ xen-hvm.c | 160 ++- xen-mapcache.c | 94 -- 8 files changed, 526 insertions(+), 53 deletions(-)
Re: [Qemu-devel] [PATCH 2/3 V3] s390: implement pci instructions
On Tue, 20 Jan 2015 10:45:41 +0100
Markus Armbruster arm...@redhat.com wrote:
This patch makes Coverity unhappy:
*** CID 1264326: Unintended sign extension (SIGN_EXTENSION)
/hw/s390x/s390-pci-inst.c: 787 in stpcifc_service_call()
781 stq_p(fib.pal, pbdev-pal);
782 stq_p(fib.iota, pbdev-g_iota);
783 stq_p(fib.aibv, pbdev-routes.adapter.ind_addr);
784 stq_p(fib.aisb, pbdev-routes.adapter.summary_addr);
785 stq_p(fib.fmb_addr, pbdev-fmb_addr);
786
CID 1264326: Unintended sign extension (SIGN_EXTENSION)
Suspicious implicit sign extension: pbdev-isc with type
unsigned char (8 bits, unsigned) is promoted in (pbdev-isc
28) | (pbdev-noi 16) to type int (32 bits, signed), then
sign-extended to type unsigned long (64 bits, unsigned). If
(pbdev-isc 28) | (pbdev-noi 16) is greater than
0x7FFF, the upper bits of the result will all be 1.
787 data = (pbdev-isc 28) | (pbdev-noi 16) |
788(pbdev-routes.adapter.ind_offset 8) | (pbdev-sum
7) |
789pbdev-routes.adapter.summary_offset;
790 stw_p(fib.data, data);
791
792 if (pbdev-fh ENABLE_BIT_OFFSET) {
There's a fix for this (and the memory leak):
http://marc.info/?l=qemu-develm=142124886620078w=2
The patch is sitting in my queue, will send with the next pile of s390x
updates.
Re: [Qemu-devel] [PATCH v0 1/2] pc: Fix DIMMs capacity calculation
On Mon, 12 Jan 2015 09:32:33 +0530
Bharata B Rao bhar...@linux.vnet.ibm.com wrote:
pc_existing_dimms_capacity() is returning DIMMs count rather than capacity.
Fix this to return the capacity. Also consider only realized devices for
capacity calculation.
Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com
Reviewed-by: Igor Mammedov imamm...@redhat.com
---
hw/i386/pc.c | 26 ++
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e07f1fa..125cf0a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1552,25 +1552,18 @@ void qemu_register_pc_machine(QEMUMachine *m)
g_free(name);
}
-static int pc_dimm_count(Object *obj, void *opaque)
-{
-int *count = opaque;
-
-if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
-(*count)++;
-}
-
-object_child_foreach(obj, pc_dimm_count, opaque);
-return 0;
-}
-
static int pc_existing_dimms_capacity(Object *obj, void *opaque)
{
Error *local_err = NULL;
uint64_t *size = opaque;
if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
-(*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
local_err);
+DeviceState *dev = DEVICE(obj);
+
+if (dev-realized) {
+(*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
+local_err);
+}
if (local_err) {
qerror_report_err(local_err);
@@ -1579,7 +1572,7 @@ static int pc_existing_dimms_capacity(Object *obj, void
*opaque)
}
}
-object_child_foreach(obj, pc_dimm_count, opaque);
+object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
return 0;
}
@@ -1623,8 +1616,9 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
if (existing_dimms_capacity + memory_region_size(mr)
machine-maxram_size - machine-ram_size) {
error_setg(local_err, not enough space, currently 0x% PRIx64
-in use of total 0x RAM_ADDR_FMT,
- existing_dimms_capacity, machine-maxram_size);
+in use of total hot pluggable 0x RAM_ADDR_FMT,
+ existing_dimms_capacity,
+ machine-maxram_size - machine-ram_size);
goto out;
}
Re: [Qemu-devel] [PATCH 0/2] virtio-blk: Switch to blk_aio_ioctl
On 20/01/2015 04:28, Fam Zheng wrote: There are user complaints on guest's unresponsiveness when ioctl is blocked due to the lost connection to backend or other issues. This series changes scsi request processing of virtio-blk to an asynchronous manner. Fam Zheng (2): virtio-blk: Pass req to virtio_blk_handle_scsi_req virtio-blk: Use blk_aio_ioctl hw/block/virtio-blk.c | 134 ++--- include/hw/virtio/virtio-blk.h | 3 - 2 files changed, 84 insertions(+), 53 deletions(-) Reviewed-by: Paolo Bonzini pbonz...@redhat.com
Re: [Qemu-devel] [PULL 00/16] Block patches
On Sat, Jan 17, 2015 at 11:41:59AM +0100, Peter Wu wrote: On Friday 16 January 2015 16:46:39 Peter Maydell wrote: CentOS5: ../block/dmg.o: In function `dmg_read_plist_xml': /home/petmay01/linaro/qemu-for-merges/block/dmg.c:414: undefined reference to `g_base64_decode_inplace' Should have paid more attention to the API docs. Can you try the following patch? It still passes 4 dmg tests for me (https://lekensteyn.nl/files/dmg-tests/). Could you also replace g_assert_false() with g_assert(!...) so it builds on Mac? Thanks, Stefan pgpuH7iTGNuOl.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 0/5] target-arm: ARM64: Adding EL1 AARCH32 guest support
On 20 January 2015 at 10:21, Sergey Fedorov serge.f...@gmail.com wrote: aarch64-softmmu/qemu-system-aarch64 -M virt -cpu cortex-a57,-aarch64 ... It seems a little confusing for me to specify '-aarch64' when forcing AArch32 execution state. Why don't just specify 'aarch32' in command line instead of '-aarch64' construction? This is one of the things we discussed during design of this patchset, and I agree the command line semantics are a bit odd. Essentially, they make sense from the PoV of the CPU object, because they're saying I want a 32-bit only CPU, ie I do not want the 64 bit feature this CPU defaults to. Specifying '+aarch32' wouldn't do anything, because the default Cortex-A57 already has 32-bit support. But from the PoV of the user, it's a bit odd. The other possible approach to this would be to have a machine model parameter for force 32 bit boot, and have this turn off the CPU 64 bit feature. Other proposals welcome; I don't like the UI we end up exposing to the user here, it's just what falls out of the natural way to model things at the CPU QOM property level. -- PMM
Re: [Qemu-devel] [v6][PATCH 07/10] xen, gfx passthrough: register a isa bridge
On Mon, Jan 19, 2015 at 05:28:40PM +0800, Tiejun Chen wrote:
Currently IGD drivers always need to access PCH by 1f.0. But we
don't want to poke that directly to get ID, and although in real
world different GPU should have different PCH. But actually the
different PCH DIDs likely map to different PCH SKUs. We do the
same thing for the GPU. For PCH, the different SKUs are going to
be all the same silicon design and implementation, just different
features turn on and off with fuses. The SW interfaces should be
consistent across all SKUs in a given family (eg LPT). But just
same features may not be supported.
Most of these different PCH features probably don't matter to the
Gfx driver, but obviously any difference in display port connections
will so it should be fine with any PCH in case of passthrough.
So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
scenarios, 0x9cc3 for BDW(Broadwell).
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
---
hw/xen/xen_pt.c | 126
1 file changed, 126 insertions(+)
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index fcc9f1c..5532d6f 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -632,6 +632,129 @@ static const MemoryListener xen_pt_io_listener = {
.priority = 10,
};
+typedef struct {
+uint16_t gpu_device_id;
+uint16_t pch_device_id;
+uint8_t pch_revision_id;
+} XenIGDDeviceIDInfo;
+
+/* In real world different GPU should have different PCH. But actually
+ * the different PCH DIDs likely map to different PCH SKUs. We do the
+ * same thing for the GPU. For PCH, the different SKUs are going to be
+ * all the same silicon design and implementation, just different
+ * features turn on and off with fuses. The SW interfaces should be
+ * consistent across all SKUs in a given family (eg LPT). But just same
+ * features may not be supported.
+ *
+ * Most of these different PCH features probably don't matter to the
+ * Gfx driver, but obviously any difference in display port connections
+ * will so it should be fine with any PCH in case of passthrough.
+ *
+ * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
+ * scenarios, 0x9cc3 for BDW(Broadwell).
+ */
+static const XenIGDDeviceIDInfo xen_igd_combo_id_infos[] = {
+/* HSW Classic */
+{0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */
+{0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */
+{0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */
+{0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */
+{0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */
+/* HSW ULT */
+{0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */
+{0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */
+{0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */
+{0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */
+{0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */
+{0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */
+/* HSW CRW */
+{0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */
+{0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */
+/* HSW Server */
+{0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */
+/* HSW SRVR */
+{0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */
+/* BSW */
+{0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */
+{0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */
+{0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */
+{0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */
+{0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */
+{0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */
+{0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */
+{0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */
+{0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */
+{0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */
+{0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */
+};
+
+static void isa_bridge_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+dc-desc= ISA bridge faked to support IGD PT;
+k-vendor_id= PCI_VENDOR_ID_INTEL;
+k-class_id = PCI_CLASS_BRIDGE_ISA;
+};
+
+static TypeInfo isa_bridge_info = {
+.name = xen-igd-passthrough-isa-bridge,
+.parent= TYPE_PCI_DEVICE,
+.instance_size = sizeof(PCIDevice),
+.class_init = isa_bridge_class_init,
+};
+
+static void xen_pt_graphics_register_types(void)
+{
+type_register_static(isa_bridge_info);
+}
+type_init(xen_pt_graphics_register_types)
+
+static void
+xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
+ XenHostPCIDevice *dev)
+{
+struct PCIDevice *pci_dev;
+int i, num;
+uint16_t gpu_dev_id, pch_dev_id = 0x;
+uint8_t pch_rev_id;
+PCIDevice *d = s-dev;
+
+if (!is_igd_vga_passthrough(dev)) {
+return;
+}
+
+
Re: [Qemu-devel] [v6][PATCH 07/10] xen, gfx passthrough: register a isa bridge
On Mon, Jan 19, 2015 at 05:28:40PM +0800, Tiejun Chen wrote:
Currently IGD drivers always need to access PCH by 1f.0. But we
don't want to poke that directly to get ID, and although in real
world different GPU should have different PCH. But actually the
different PCH DIDs likely map to different PCH SKUs. We do the
same thing for the GPU. For PCH, the different SKUs are going to
be all the same silicon design and implementation, just different
features turn on and off with fuses. The SW interfaces should be
consistent across all SKUs in a given family (eg LPT). But just
same features may not be supported.
Most of these different PCH features probably don't matter to the
Gfx driver, but obviously any difference in display port connections
will so it should be fine with any PCH in case of passthrough.
So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
scenarios, 0x9cc3 for BDW(Broadwell).
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
---
hw/xen/xen_pt.c | 126
1 file changed, 126 insertions(+)
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index fcc9f1c..5532d6f 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -632,6 +632,129 @@ static const MemoryListener xen_pt_io_listener = {
.priority = 10,
};
+typedef struct {
+uint16_t gpu_device_id;
+uint16_t pch_device_id;
+uint8_t pch_revision_id;
+} XenIGDDeviceIDInfo;
+
+/* In real world different GPU should have different PCH. But actually
+ * the different PCH DIDs likely map to different PCH SKUs. We do the
+ * same thing for the GPU. For PCH, the different SKUs are going to be
+ * all the same silicon design and implementation, just different
+ * features turn on and off with fuses. The SW interfaces should be
+ * consistent across all SKUs in a given family (eg LPT). But just same
+ * features may not be supported.
+ *
+ * Most of these different PCH features probably don't matter to the
+ * Gfx driver, but obviously any difference in display port connections
+ * will so it should be fine with any PCH in case of passthrough.
+ *
+ * So currently use one PCH version, 0x8c4e, to cover all HSW(Haswell)
+ * scenarios, 0x9cc3 for BDW(Broadwell).
+ */
+static const XenIGDDeviceIDInfo xen_igd_combo_id_infos[] = {
+/* HSW Classic */
+{0x0402, 0x8c4e, 0x04}, /* HSWGT1D, HSWD_w7 */
+{0x0406, 0x8c4e, 0x04}, /* HSWGT1M, HSWM_w7 */
+{0x0412, 0x8c4e, 0x04}, /* HSWGT2D, HSWD_w7 */
+{0x0416, 0x8c4e, 0x04}, /* HSWGT2M, HSWM_w7 */
+{0x041E, 0x8c4e, 0x04}, /* HSWGT15D, HSWD_w7 */
+/* HSW ULT */
+{0x0A06, 0x8c4e, 0x04}, /* HSWGT1UT, HSWM_w7 */
+{0x0A16, 0x8c4e, 0x04}, /* HSWGT2UT, HSWM_w7 */
+{0x0A26, 0x8c4e, 0x06}, /* HSWGT3UT, HSWM_w7 */
+{0x0A2E, 0x8c4e, 0x04}, /* HSWGT3UT28W, HSWM_w7 */
+{0x0A1E, 0x8c4e, 0x04}, /* HSWGT2UX, HSWM_w7 */
+{0x0A0E, 0x8c4e, 0x04}, /* HSWGT1ULX, HSWM_w7 */
+/* HSW CRW */
+{0x0D26, 0x8c4e, 0x04}, /* HSWGT3CW, HSWM_w7 */
+{0x0D22, 0x8c4e, 0x04}, /* HSWGT3CWDT, HSWD_w7 */
+/* HSW Server */
+{0x041A, 0x8c4e, 0x04}, /* HSWSVGT2, HSWD_w7 */
+/* HSW SRVR */
+{0x040A, 0x8c4e, 0x04}, /* HSWSVGT1, HSWD_w7 */
+/* BSW */
+{0x1606, 0x9cc3, 0x03}, /* BDWULTGT1, BDWM_w7 */
+{0x1616, 0x9cc3, 0x03}, /* BDWULTGT2, BDWM_w7 */
+{0x1626, 0x9cc3, 0x03}, /* BDWULTGT3, BDWM_w7 */
+{0x160E, 0x9cc3, 0x03}, /* BDWULXGT1, BDWM_w7 */
+{0x161E, 0x9cc3, 0x03}, /* BDWULXGT2, BDWM_w7 */
+{0x1602, 0x9cc3, 0x03}, /* BDWHALOGT1, BDWM_w7 */
+{0x1612, 0x9cc3, 0x03}, /* BDWHALOGT2, BDWM_w7 */
+{0x1622, 0x9cc3, 0x03}, /* BDWHALOGT3, BDWM_w7 */
+{0x162B, 0x9cc3, 0x03}, /* BDWHALO28W, BDWM_w7 */
+{0x162A, 0x9cc3, 0x03}, /* BDWGT3WRKS, BDWM_w7 */
+{0x162D, 0x9cc3, 0x03}, /* BDWGT3SRVR, BDWM_w7 */
+};
+
+static void isa_bridge_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+dc-desc= ISA bridge faked to support IGD PT;
+k-vendor_id= PCI_VENDOR_ID_INTEL;
+k-class_id = PCI_CLASS_BRIDGE_ISA;
+};
+
+static TypeInfo isa_bridge_info = {
+.name = xen-igd-passthrough-isa-bridge,
+.parent= TYPE_PCI_DEVICE,
+.instance_size = sizeof(PCIDevice),
+.class_init = isa_bridge_class_init,
+};
+
+static void xen_pt_graphics_register_types(void)
+{
+type_register_static(isa_bridge_info);
+}
+type_init(xen_pt_graphics_register_types)
+
+static void
+xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
+ XenHostPCIDevice *dev)
+{
I suggest this implementation, and the table, are moved
to the same file where igd-passthrough-isa-bridge
is implemented. The function can get PCIDevice *d, this way
it's not xen specific.
+struct PCIDevice *pci_dev;
pls rename
Re: [Qemu-devel] [PATCH RFC v6 12/20] virtio: disallow late feature changes for virtio-1
On Fri, Dec 12, 2014 at 12:25:47PM +0100, Thomas Huth wrote: On Fri, 12 Dec 2014 12:18:25 +0100 Cornelia Huck cornelia.h...@de.ibm.com wrote: On Fri, 12 Dec 2014 11:55:38 +0100 Thomas Huth th...@linux.vnet.ibm.com wrote: On Thu, 11 Dec 2014 14:25:14 +0100 Cornelia Huck cornelia.h...@de.ibm.com wrote: For virtio-1 devices, the driver must not attempt to set feature bits after it set FEATURES_OK in the device status. Simply reject it in that case. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/virtio/virtio.c | 16 ++-- include/hw/virtio/virtio.h |2 ++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 57190ba..a3dd67b 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -978,7 +978,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f) vmstate_save_state(f, vmstate_virtio, vdev); } -int virtio_set_features(VirtIODevice *vdev, uint64_t val) +static int __virtio_set_features(VirtIODevice *vdev, uint64_t val) Maybe avoid the double underscores here? But unfortunately, I also fail to come up with a better suggestion for a name here ... virtio_set_features_nocheck()? Sounds ok to me. This function is only called within virtio.c anyway... Right, so the double underscores should be ok here, too. (I still do not like them very much, but that's just my personal taste in this case) C99 7.1.3 Reserved identifiers says: All identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use [by the standard library] You can use a trailing underscore or useless word like do, e.g. virtio_do_set_features(), for internal functions. pgp7jk_2ZaG1G.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v2 7/7] sdl2: add support for display rendering using opengl.
Hi,
+void sdl2_gl_switch(DisplayChangeListener *dcl,
+DisplaySurface *new_surface)
+{
+struct sdl2_console *scon = container_of(dcl, struct sdl2_console,
dcl);
+DisplaySurface *old_surface = scon-surface;
+
+assert(scon-opengl);
+
+SDL_GL_MakeCurrent(scon-real_window, scon-winctx);
+surface_gl_destroy_texture(scon-gls, scon-surface);
Same question as for v1: Can a surface be in use by multiple DCLs?
Oops, I remember that comment for the last series, forgot to answer ...
Yes. Old surface is released after notifying all DCLs about the new one
(see dpy_gfx_replace_surface() in console.c), so the old is still valid
at this point. That way refcounting should not be needed in most cases.
If a DCL needs the old surface image stay around after it returns from
the switch callback (say due to threads still holding pointers to it) it
can grab a reference on the pixman image backing the surface:
pixman_image_ref(surface-image). The surfaces itself are not reference
counted.
+} else if (old_surface
+ ((surface_width(old_surface) !=
surface_width(new_surface)) ||
+(surface_height(old_surface) !=
surface_height(new_surface {
And as in v1: If the window is scaled, this will reset the scaling to
100 %, which is fine. However, if the new surface has the same
dimensions as the old surface, the window will not be scaled. That would
seem strange to me (why is the scaling reset for some surface changes
but not for others?).
Yea, there are some corner cases. But they are not specific to opengl,
they happen in sdl2-2d mode too, so that is something for another patch
series ...
cheers,
Gerd
Re: [Qemu-devel] [v6][PATCH 03/10] piix: create host bridge to passthrough
On 01/20/2015 10:52 AM, Chen, Tiejun wrote:
On 2015/1/19 19:40, Gerd Hoffmann wrote:
On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote:
+static void xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
+ void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc-desc = IGD PT XEN Host bridge;
+}
IMO xen naming should go away here too.
I would agree with this.
In fact, this piece of code could possibly be used by:
a) IGD passthru for Xen and KVM, and/or:
b) IGD Mediated passthru for Xen and KVM, i.e. XenGT/KVMGT
So it looks better if have xen naming purged :)
Its easy to do but we need to wait KvmGT guys' response, so now it makes
sense to leave xen as a prefix since this just work in xen side.
Thanks
Tiejun
--
Thanks,
Jike
Re: [Qemu-devel] [PATCH] target-mips: Clean up switch fall through after commit fecd264
Peter Maydell peter.mayd...@linaro.org writes: On 20 January 2015 at 09:59, Markus Armbruster arm...@redhat.com wrote: Commit fecd264 added a number of fall-throughs, but neglected to properly document them as intentional. Commit d922445 cleaned that up for many, but not all cases. Take care of the remaining ones. Spotted by Coverity. Signed-off-by: Markus Armbruster arm...@redhat.com --- target-mips/translate.c | 4 1 file changed, 4 insertions(+) diff --git a/target-mips/translate.c b/target-mips/translate.c index e9d86b2..8abc12b 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -18729,6 +18729,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) case OPC_SWL: case OPC_SWR: check_insn_opc_removed(ctx, ISA_MIPS32R6); + /* fall through */ Indent here seems to be out by one? The others look OK. Sorry about that. Fix up on commit, or would you like a respin?
Re: [Qemu-devel] [PATCH 2/2] m48t59: add mem_base value to m48t59_init_isa()
On Mon, Jan 19, 2015 at 9:03 PM, Andreas Färber afaer...@suse.de wrote: Am 19.01.2015 um 16:22 schrieb Artyom Tarasenko: On Mon, Jan 19, 2015 at 4:01 PM, Andreas Färber afaer...@suse.de wrote: Am 19.01.2015 um 13:57 schrieb Artyom Tarasenko: On Mon, Jan 19, 2015 at 1:45 PM, Paolo Bonzini pbonz...@redhat.com wrote: On 19/01/2015 12:35, Mark Cave-Ayland wrote: Similar to m48t59_init(), add a mem_base value so that NVRAM can be mapped via MMIO rather than ioport if required. Signed-off-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk --- Is it really ISA if it's MMIO? In other words, why can't this be a sysbus device? On physical machines it's EBus, which is pretty much like 8-bit ISA. So, I think modelling it as ISA is closer to to the reality. But out of curiosity, would it be possible to have a sysbus device somewhere in a middle of PCI space? [...] Why would you want to use a SysBusDevice in the first place? Ask Paolo. :-) For me it's only important to have a MMIO device in the proper address range. I previously discussed with Mark that it should be an EBusDevice, not an ISADevice or SysBusDevice. Interesting. I can't find this discussion in the list archive. Hm, am I mixing that up with SBus then? There were some helper functions related to ROM loading being added as context to my suggestion that I thought could become class fields. Yes, for SBus it totally makes sense. Do you suggest to create EBusDevices for all ISA devices (serial, parallel, keyboard, floppy) used in sun4u, or only for m48t59? What would be the advantage of using EBusDevice over ISADevice? For all devices that are in fact EBus devices. The general idea is encapsulation and abstraction - hiding the implementation detail of whether it is internally an ISADevice or something else. Such a patch should be quite trivial. Similarly it gives helper functions and potential class methods a natural place to live. Yes, the patch is trivial. But EBus is nothing else but an ISA bus with 8 data lines. To me it looks like the bus width is an implementation detail we can hide. (It's not documented what should happen if an EBus device is accessed with a non 8-bit width. I tried 16 bit access on a physical machine and it seemed to work). Currently we can just use all the ISA devices unmodified if we like. With EBus we would only be able to use a subset of ISA devices, no? Artyom -- Regards, Artyom Tarasenko SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu
Re: [Qemu-devel] [PATCH v3 4/5] migration: Append JSON description of migration stream
On (Fri) 26 Dec 2014 [15:42:47], Alexander Graf wrote: One of the annoyances of the current migration format is the fact that it's not self-describing. In fact, it's not properly describing at all. Some code randomly scattered throughout QEMU elaborates roughly how to read and write a stream of bytes. We discussed an idea during KVM Forum 2013 to add a JSON description of the migration protocol itself to the migration stream. This patch adds a section after the VM_END migration end marker that contains description data on what the device sections of the stream are composed of. Can you add a note saying why this is safe for migrations to qemu versions that don't expect this new json data? Amit
Re: [Qemu-devel] [PATCH RFC v6 06/20] virtio: endianness checks for virtio 1.0 devices
On Thu, Dec 11, 2014 at 02:25:08PM +0100, Cornelia Huck wrote: Add code that checks for the VERSION_1 feature bit in order to make decisions about the device's endianness. This allows us to support transitional devices. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/virtio/virtio.c|6 +- include/hw/virtio/virtio-access.h |4 include/hw/virtio/virtio.h|8 ++-- 3 files changed, 15 insertions(+), 3 deletions(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgpyP3qS6pI8D.pgp Description: PGP signature
Re: [Qemu-devel] [RESEND PATCH v1 0/5] Common unplug and unplug request cb for memory and CPU hot-unplug.
On Tue, Jan 20, 2015 at 11:03:02AM +0100, Igor Mammedov wrote: On Mon, 19 Jan 2015 23:29:37 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jan 07, 2015 at 02:49:40PM +0800, Tang Chen wrote: Memory and CPU hot unplug are both asynchronize procedures. When the unplug operation happens, unplug request cb is called first. And when ghest OS finished handling unplug, unplug cb will be called to do the real removal of device. They both need pc-machine, piix4 and ich9 unplug and unplug request cb. So this patch set introduces these commom functions as part1, and memory and CPU hot-unplug will come soon as part 2 and 3. This patch-set is based on QEmu 2.2 OK, Igor - you only have comments for the commit log? I take this as implicit ack of the patches? If so pls let me know. Yes, patches themselves are good. Can fixup commit messages and add author SoBs to them, before applying or should we wait for respin? I didn't notice signatures were missing. This is required for inclusion. Tang Chen, please rebase on top of my pci branch, but also, we don't include code of uncertain origin. So - if you can certify the below: Developer's Certificate of Origin 1.1 By making a contribution to this project, I certify that: (a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or (b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or (c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it. (d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved. then you add a line at the end of each commit message saying Signed-off-by: Random J Developer ran...@developer.example.org for each developer who participated in writing this code. Tang Chen (5): acpi, pc: Add hotunplug request cb for pc machine. acpi, ich9: Add hotunplug request cb for ich9. acpi, pc: Add unplug cb for pc machine. acpi, ich9: Add unplug cb for ich9. acpi, piix4: Add unplug cb for piix4. hw/acpi/ich9.c | 14 ++ hw/acpi/piix4.c| 8 hw/i386/pc.c | 16 hw/isa/lpc_ich9.c | 14 -- include/hw/acpi/ich9.h | 4 5 files changed, 54 insertions(+), 2 deletions(-) -- 1.8.4.2
Re: [Qemu-devel] [v6][PATCH 08/10] xen, gfx passthrough: support Intel IGD passthrough with VT-D
On Mon, Jan 19, 2015 at 05:28:41PM +0800, Tiejun Chen wrote:
Some registers of Intel IGD are mapped in host bridge, so it needs to
passthrough these registers of physical host bridge to guest because
emulated host bridge in guest doesn't have these mappings.
Signed-off-by: Tiejun Chen tiejun.c...@intel.com
Signed-off-by: Yang Zhang yang.z.zh...@intel.com
---
hw/pci-host/piix.c | 3 ++
hw/xen/xen_pt.h | 1 +
hw/xen/xen_pt_graphics.c | 72
3 files changed, 76 insertions(+)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 1468961..0a5a4c7 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -34,6 +34,7 @@
#include sysemu/sysemu.h
#include hw/i386/ioapic.h
#include qapi/visitor.h
+#include hw/xen/xen_pt.h
/*
* I440FX chipset data sheet.
@@ -733,8 +734,10 @@ static void
xen_igd_passthrough_i440fx_class_init(ObjectClass *klass,
void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
dc-desc = IGD PT XEN Host bridge;
+k-config_read = xen_igd_pci_read;
}
static const TypeInfo xen_igd_passthrough_i440fx_info = {
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 0aa5a93..94cde4a 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -5,6 +5,7 @@
#include hw/xen/xen_common.h
#include hw/pci/pci.h
#include xen-host-pci-device.h
+uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len);
void xen_pt_log(const PCIDevice *d, const char *f, ...) GCC_FMT_ATTR(2, 3);
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index 3232296..227089b 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -4,6 +4,7 @@
#include xen_pt.h
#include xen-host-pci-device.h
#include hw/xen/xen_backend.h
+#include hw/pci/pci_bus.h
typedef struct VGARegion {
int type; /* Memory or port I/O */
@@ -188,3 +189,74 @@ int xen_pt_setup_vga(XenPCIPassthroughState *s,
XenHostPCIDevice *dev)
cpu_physical_memory_rw(0xc, bios, bios_size, 1);
return 0;
}
+
+/*
+ * Currently we just pass this physical host bridge for IGD, 00:02.0.
+ *
+ * Here pci_dev is just that host bridge, so we have to get that real
+ * passthrough device by that given devfn to avoid other devices access.
+ */
+static int is_igd_passthrough(PCIDevice *pci_dev)
+{
+PCIDevice *f = pci_dev-bus-devices[PCI_DEVFN(2, 0)];
+if (pci_dev-bus-devices[PCI_DEVFN(2, 0)]) {
+XenPCIPassthroughState *s = DO_UPCAST(XenPCIPassthroughState, dev,
f);
+return (is_igd_vga_passthrough(s-real_device)
+ (s-real_device.vendor_id == PCI_VENDOR_ID_INTEL));
+} else {
+return 0;
+}
+}
+
+uint32_t xen_igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len)
+{
+XenHostPCIDevice dev;
+uint32_t val;
+int r;
+
+/* IGD read/write is through the host bridge.
+ */
+assert(pci_dev-devfn == 0x00);
+
+if (!is_igd_passthrough(pci_dev)) {
+goto read_default;
+}
+
+/* Just work for the i915 driver. */
+switch (config_addr) {
+case 0x08:/* revision id */
+case 0x2c:/* sybsystem vendor id */
+case 0x2e:/* sybsystem id */
+case 0x50:/* SNB: processor graphics control register */
+case 0x52:/* processor graphics control register */
+case 0xa0:/* top of memory */
Is this host physical memory? If yes how can using it in guest work?
+case 0xa4:/* SNB: graphics base of stolen memory */
+case 0xa8:/* SNB: base of GTT stolen memory */
Same question for above two.
+break;
+default:
+/* Just gets the emulated values. */
+goto read_default;
+}
+
+/* Host read */
+r = xen_host_pci_device_get(dev, 0, 0, 0, 0);
+if (r) {
+goto err_out;
+}
+
+r = xen_host_pci_get_block(dev, config_addr, (uint8_t *)val, len);
+if (r) {
+goto err_out;
+}
+
+xen_host_pci_device_put(dev);
+
+return val;
+
+read_default:
+return pci_default_read_config(pci_dev, config_addr, len);
+
+err_out:
+XEN_PT_ERR(pci_dev, Can't get pci_dev_host_bridge\n);
+return -1;
+}
Do any of the above registers change with time?
Does it work if we just read them when device is created
and put in dev-config?
--
1.9.1
[Qemu-devel] [PULL 2/4] Xen: Use the ioreq-server API when available
From: Paul Durrant paul.durr...@citrix.com
The ioreq-server API added to Xen 4.5 offers better security than
the existing Xen/QEMU interface because the shared pages that are
used to pass emulation request/results back and forth are removed
from the guest's memory space before any requests are serviced.
This prevents the guest from mapping these pages (they are in a
well known location) and attempting to attack QEMU by synthesizing
its own request structures. Hence, this patch modifies configure
to detect whether the API is available, and adds the necessary
code to use the API if it is.
Signed-off-by: Paul Durrant paul.durr...@citrix.com
Acked-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
---
configure | 29 ++
include/hw/xen/xen_common.h | 223 +++
trace-events|9 ++
xen-hvm.c | 160 ++-
4 files changed, 399 insertions(+), 22 deletions(-)
diff --git a/configure b/configure
index 7539645..5ea1014 100755
--- a/configure
+++ b/configure
@@ -1877,6 +1877,32 @@ int main(void) {
xc_gnttab_open(NULL, 0);
xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
+ xc_hvm_create_ioreq_server(xc, 0, 0, NULL);
+ return 0;
+}
+EOF
+ compile_prog $xen_libs
+then
+xen_ctrl_version=450
+xen=yes
+
+ elif
+ cat $TMPC EOF
+#include xenctrl.h
+#include xenstore.h
+#include stdint.h
+#include xen/hvm/hvm_info_table.h
+#if !defined(HVM_MAX_VCPUS)
+# error HVM_MAX_VCPUS not defined
+#endif
+int main(void) {
+ xc_interface *xc;
+ xs_daemon_open();
+ xc = xc_interface_open(0, 0, 0);
+ xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
+ xc_gnttab_open(NULL, 0);
+ xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
+ xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
return 0;
}
EOF
@@ -4283,6 +4309,9 @@ if test -n $sparc_cpu; then
echo Target Sparc Arch $sparc_cpu
fi
echo xen support $xen
+if test $xen = yes ; then
+ echo xen ctrl version $xen_ctrl_version
+fi
echo brlapi support$brlapi
echo bluez support$bluez
echo Documentation $docs
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 95612a4..519696f 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -16,7 +16,9 @@
#include hw/hw.h
#include hw/xen/xen.h
+#include hw/pci/pci.h
#include qemu/queue.h
+#include trace.h
/*
* We don't support Xen prior to 3.3.0.
@@ -179,4 +181,225 @@ static inline int xen_get_vmport_regs_pfn(XenXC xc,
domid_t dom,
}
#endif
+/* Xen before 4.5 */
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION 450
+
+#ifndef HVM_PARAM_BUFIOREQ_EVTCHN
+#define HVM_PARAM_BUFIOREQ_EVTCHN 26
+#endif
+
+#define IOREQ_TYPE_PCI_CONFIG 2
+
+typedef uint32_t ioservid_t;
+
+static inline void xen_map_memory_section(XenXC xc, domid_t dom,
+ ioservid_t ioservid,
+ MemoryRegionSection *section)
+{
+}
+
+static inline void xen_unmap_memory_section(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+MemoryRegionSection *section)
+{
+}
+
+static inline void xen_map_io_section(XenXC xc, domid_t dom,
+ ioservid_t ioservid,
+ MemoryRegionSection *section)
+{
+}
+
+static inline void xen_unmap_io_section(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+MemoryRegionSection *section)
+{
+}
+
+static inline void xen_map_pcidev(XenXC xc, domid_t dom,
+ ioservid_t ioservid,
+ PCIDevice *pci_dev)
+{
+}
+
+static inline void xen_unmap_pcidev(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+PCIDevice *pci_dev)
+{
+}
+
+static inline int xen_create_ioreq_server(XenXC xc, domid_t dom,
+ ioservid_t *ioservid)
+{
+return 0;
+}
+
+static inline void xen_destroy_ioreq_server(XenXC xc, domid_t dom,
+ioservid_t ioservid)
+{
+}
+
+static inline int xen_get_ioreq_server_info(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+xen_pfn_t *ioreq_pfn,
+xen_pfn_t *bufioreq_pfn,
+evtchn_port_t *bufioreq_evtchn)
+{
+unsigned long param;
+int rc;
+
+rc = xc_get_hvm_param(xc, dom, HVM_PARAM_IOREQ_PFN, param);
+if (rc 0) {
+fprintf(stderr, failed to get HVM_PARAM_IOREQ_PFN\n);
+return -1;
+}
+
+*ioreq_pfn = param;
+
+rc = xc_get_hvm_param(xc, dom,
[Qemu-devel] [PULL 1/4] Add device listener interface
From: Paul Durrant paul.durr...@citrix.com
The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device
models explicitly register with Xen for config space accesses. This patch
adds a listener interface into qdev-core which can be used by the Xen
interface code to monitor for arrival and departure of PCI devices.
Signed-off-by: Paul Durrant paul.durr...@citrix.com
Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
---
hw/core/qdev.c | 53 +++
include/hw/qdev-core.h | 10 +
include/qemu/typedefs.h |1 +
3 files changed, 64 insertions(+)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 901f289..2eacac0 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -189,6 +189,56 @@ int qdev_init(DeviceState *dev)
return 0;
}
+static QTAILQ_HEAD(device_listeners, DeviceListener) device_listeners
+= QTAILQ_HEAD_INITIALIZER(device_listeners);
+
+enum ListenerDirection { Forward, Reverse };
+
+#define DEVICE_LISTENER_CALL(_callback, _direction, _args...) \
+do { \
+DeviceListener *_listener;\
+ \
+switch (_direction) { \
+case Forward: \
+QTAILQ_FOREACH(_listener, device_listeners, link) { \
+if (_listener-_callback) { \
+_listener-_callback(_listener, ##_args); \
+} \
+} \
+break;\
+case Reverse: \
+QTAILQ_FOREACH_REVERSE(_listener, device_listeners, \
+ device_listeners, link) { \
+if (_listener-_callback) { \
+_listener-_callback(_listener, ##_args); \
+} \
+} \
+break;\
+default: \
+abort(); \
+} \
+} while (0)
+
+static int device_listener_add(DeviceState *dev, void *opaque)
+{
+DEVICE_LISTENER_CALL(realize, Forward, dev);
+
+return 0;
+}
+
+void device_listener_register(DeviceListener *listener)
+{
+QTAILQ_INSERT_TAIL(device_listeners, listener, link);
+
+qbus_walk_children(sysbus_get_default(), NULL, NULL, device_listener_add,
+ NULL, NULL);
+}
+
+void device_listener_unregister(DeviceListener *listener)
+{
+QTAILQ_REMOVE(device_listeners, listener, link);
+}
+
static void device_realize(DeviceState *dev, Error **errp)
{
DeviceClass *dc = DEVICE_GET_CLASS(dev);
@@ -994,6 +1044,8 @@ static void device_set_realized(Object *obj, bool value,
Error **errp)
goto fail;
}
+DEVICE_LISTENER_CALL(realize, Forward, dev);
+
hotplug_ctrl = qdev_get_hotplug_handler(dev);
if (hotplug_ctrl) {
hotplug_handler_plug(hotplug_ctrl, dev, local_err);
@@ -1035,6 +1087,7 @@ static void device_set_realized(Object *obj, bool value,
Error **errp)
dc-unrealize(dev, local_errp);
}
dev-pending_deleted_event = true;
+DEVICE_LISTENER_CALL(unrealize, Reverse, dev);
}
if (local_err != NULL) {
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 589bbe7..15a226f 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -165,6 +165,12 @@ struct DeviceState {
int alias_required_for_version;
};
+struct DeviceListener {
+void (*realize)(DeviceListener *listener, DeviceState *dev);
+void (*unrealize)(DeviceListener *listener, DeviceState *dev);
+QTAILQ_ENTRY(DeviceListener) link;
+};
+
#define TYPE_BUS bus
#define BUS(obj) OBJECT_CHECK(BusState, (obj), TYPE_BUS)
#define BUS_CLASS(klass) OBJECT_CLASS_CHECK(BusClass, (klass), TYPE_BUS)
@@ -376,4 +382,8 @@ static inline bool qbus_is_hotpluggable(BusState *bus)
{
return bus-hotplug_handler;
}
+
+void device_listener_register(DeviceListener *listener);
+void device_listener_unregister(DeviceListener *listener);
+
#endif
diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
index f2bbaaf..cde3314 100644
--- a/include/qemu/typedefs.h
+++ b/include/qemu/typedefs.h
@@ -17,6 +17,7 @@ typedef struct BusState BusState;
typedef struct CharDriverState CharDriverState;
typedef struct
Re: [Qemu-devel] [PATCH v2 1/4] pc: append ssdt-misc.dsl to the DSDT
On Tue, 20 Jan 2015 12:35:47 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Jan 20, 2015 at 10:59:43AM +0100, Igor Mammedov wrote: On Mon, 19 Jan 2015 21:29:57 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Jan 19, 2015 at 06:26:55PM +0100, Paolo Bonzini wrote: On 19/01/2015 18:14, Igor Mammedov wrote: I'm fine with moving SMC out of the per-machine-type AML, should be a separate patch anyway. But patch-able SMC being in DSDT is our mistake that we allowed it to slip there and should be better moved to SSDT rather than staying in DSDT and making thing more complex. It's also candidate for trimming, i.e. dropping it from tables altogether if device is not present in QEMU, same applies to _S[34] Packages when respective features are disabled and to PEVT device template. Yes, trimming is better than putting it in the DSDT, at least for simple devices such as SMC and pvpanic. So are we dropping 1-2/4 from this series? I need to know on top of what to rebase. I'll take care of moving SMC to SSDT. simpler. However, it also complicates backwards compatibility, so merge it with the DSDT. What are these complications? The complication arises if we want to make the SSDT exactly the same for all QEMU versions, given a (machine type, command line) pair. Then you either cannot do any change to ssdt-misc, or you have to keep different copies for each machine type. With resizable ROM blobs in master, there shouldn't be an issue with migration in new QEMU versions if size of SSDT changes. There is only a very small issue that remains (the RSDP pointer is wrong if the size changes), Yes - for new machine types I'll send a patch to put it in memory. For old ones - there's a race, and it's painful to fix. If we do want to try fixing it, one solution is to fail migration if attempted before rsdp is shadowed. Useful? There were my patches on list that move RSDT at the start of blob, which fixes issue for new machine types. I don't see the point - IMO for new machine types, we can just put RSDP in a memory region, have it migrated. you mean to put it into ROM blob, that should will cover not only migration issue but also reboot after bridge hotplug, since updated RSDP will be used. That patches however weren't doing good job for old machine types. I can respin that series fixing new machines and we can fix old machines in separate patch later. I don't think it's worth it since I don't see an easy way for old machine types. A harder way would be to allow rom files to share an MR. We could then stick RSDP at the tail of the MR, and look for it on incoming migration: if there, fix it up. Needs reworking of rom_add_blob API. so we probably should apply anyway the patch of mine that allows the DSDT size to change; and we probably should pay attention to SSDT, and version it. (Let's just ignore the SSDT was exactly what I feared when I disagreed with putting in resizable ROM blobs first. But now that it's in, I cannot really argue otherwise). I don't have a strong opinion here. you guys arrive at a rough consensus.:w So question is if we still need SSDT version-ing and per machine type SSDT compatibility? /it's better not to do version-ing at all if it could be avoided, due to maintenance headache it brings along/ I'm okay with re-evaluating that after your patches go in. Paolo
Re: [Qemu-devel] [PATCH v2 1/4] pc: append ssdt-misc.dsl to the DSDT
On Tue, Jan 20, 2015 at 01:41:16PM +0100, Igor Mammedov wrote: On Tue, 20 Jan 2015 12:35:47 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Tue, Jan 20, 2015 at 10:59:43AM +0100, Igor Mammedov wrote: On Mon, 19 Jan 2015 21:29:57 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Mon, Jan 19, 2015 at 06:26:55PM +0100, Paolo Bonzini wrote: On 19/01/2015 18:14, Igor Mammedov wrote: I'm fine with moving SMC out of the per-machine-type AML, should be a separate patch anyway. But patch-able SMC being in DSDT is our mistake that we allowed it to slip there and should be better moved to SSDT rather than staying in DSDT and making thing more complex. It's also candidate for trimming, i.e. dropping it from tables altogether if device is not present in QEMU, same applies to _S[34] Packages when respective features are disabled and to PEVT device template. Yes, trimming is better than putting it in the DSDT, at least for simple devices such as SMC and pvpanic. So are we dropping 1-2/4 from this series? I need to know on top of what to rebase. I'll take care of moving SMC to SSDT. simpler. However, it also complicates backwards compatibility, so merge it with the DSDT. What are these complications? The complication arises if we want to make the SSDT exactly the same for all QEMU versions, given a (machine type, command line) pair. Then you either cannot do any change to ssdt-misc, or you have to keep different copies for each machine type. With resizable ROM blobs in master, there shouldn't be an issue with migration in new QEMU versions if size of SSDT changes. There is only a very small issue that remains (the RSDP pointer is wrong if the size changes), Yes - for new machine types I'll send a patch to put it in memory. For old ones - there's a race, and it's painful to fix. If we do want to try fixing it, one solution is to fail migration if attempted before rsdp is shadowed. Useful? There were my patches on list that move RSDT at the start of blob, which fixes issue for new machine types. I don't see the point - IMO for new machine types, we can just put RSDP in a memory region, have it migrated. you mean to put it into ROM blob, that should will cover not only migration issue but also reboot after bridge hotplug, since updated RSDP will be used. Exactly. We can reuse the original rom blob but it's tricky given existing APIs. That patches however weren't doing good job for old machine types. I can respin that series fixing new machines and we can fix old machines in separate patch later. I don't think it's worth it since I don't see an easy way for old machine types. A harder way would be to allow rom files to share an MR. We could then stick RSDP at the tail of the MR, and look for it on incoming migration: if there, fix it up. Needs reworking of rom_add_blob API. so we probably should apply anyway the patch of mine that allows the DSDT size to change; and we probably should pay attention to SSDT, and version it. (Let's just ignore the SSDT was exactly what I feared when I disagreed with putting in resizable ROM blobs first. But now that it's in, I cannot really argue otherwise). I don't have a strong opinion here. you guys arrive at a rough consensus.:w So question is if we still need SSDT version-ing and per machine type SSDT compatibility? /it's better not to do version-ing at all if it could be avoided, due to maintenance headache it brings along/ I'm okay with re-evaluating that after your patches go in. Paolo
[Qemu-devel] [PATCH] apic: do not dereference pointer before it is checked for NULL
Right now you only get to apic_init_reset if you have an APIC
(do_cpu_init is reached only if CPU_INTERRUPT_INIT is set and
that only happens in hw/intc/apic.c). However, this is wrong
because for example a port 92 or keyboard controller reset is
really an INIT, and that can happen also with no APIC. So
keep the check and fix the error that Coverity reported.
Reported-by: Markus Armbruster arm...@redhat.com
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
hw/intc/apic_common.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index d9bb188..0858b45 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -177,13 +177,14 @@ bool apic_next_timer(APICCommonState *s, int64_t
current_time)
void apic_init_reset(DeviceState *dev)
{
-APICCommonState *s = APIC_COMMON(dev);
-APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
+APICCommonState *s;
+APICCommonClass *info;
int i;
-if (!s) {
+if (!dev) {
return;
}
+s = APIC_COMMON(dev);
s-tpr = 0;
s-spurious_vec = 0xff;
s-log_dest = 0;
@@ -208,6 +209,7 @@ void apic_init_reset(DeviceState *dev)
}
s-timer_expiry = -1;
+info = APIC_COMMON_GET_CLASS(s);
if (info-reset) {
info-reset(s);
}
--
1.8.3.1
Re: [Qemu-devel] [PATCH v3 0/5] Migration Deciphering aid
On 20.01.15 11:31, Amit Shah wrote: On (Fri) 26 Dec 2014 [15:42:43], Alexander Graf wrote: Migration is a black hole to most people. One of the biggest reasons for this is that its protocol is a secret, undocumented sauce of code rolling around random parts of the QEMU code base. But what if we simply exposed the description of how the format looks like alongside the actual migration stream? This is what this patch set does. It adds a new section that comes after the end of stream marker (so that it doesn't slow down migration) that contains a JSON description of the device state description. Along with this patch set also comes a python script that can read said JSON from a migration dump and decipher the device state and ram contents of the migration dump using it. With this, you can now fully examine all glorious details that go over the wire when virtual machine state gets dumped, such as during live migration. We discussed the approach taken here during KVM Forum 2013. Originally, my idea was to include a special device that contains the JSON data which can be enabled on demand. Anthony suggested however to just always include the description data after the end marker which I think is a great idea. Example decoded migration: http://csgraf.de/mig/mig.txt Example migration description: http://csgraf.de/mig/mig.desc.txt Presentation: https://www.youtube.com/watch?v=iq1x40Qsrew Slides: https://www.dropbox.com/s/otp2pk2n3g087zp/Live%20Migration.pdf Nice to finally see this! I guess you have a v4 coming soon? Yeah, I was just waiting on a bit more review :) Alex
Re: [Qemu-devel] [v6][PATCH 01/10] i440fx: make types configurable at run-time
On 01/20/2015 04:25 PM, Gerd Hoffmann wrote: On Di, 2015-01-20 at 10:48 +0800, Chen, Tiejun wrote: On 2015/1/19 19:36, Gerd Hoffmann wrote: On Mo, 2015-01-19 at 17:28 +0800, Tiejun Chen wrote: From: Michael S. Tsirkin m...@redhat.com Xen wants to supply a different pci and host devices, inheriting i440fx devices. Make types configurable. Description is misleading, this isn't about xen but about IGD Its about IGD passthrough in Xen side. As far I can see the only really xen specific stuff here is the pci pass-through bits, i.e. how we handle the IGD device itself. The northbridge isa-bridge emulation extensions needed for IGD should be pretty much common for IGD passthough on xen, IGD passthrough on kvm (vfio) and IGD virtualization (xengt+kvmgt). This is exactly what I proposed in another thread :) cheers, Gerd -- Thanks, Jike
Re: [Qemu-devel] [PATCH 2/3 V3] s390: implement pci instructions
Cornelia Huck cornelia.h...@de.ibm.com writes:
On Tue, 20 Jan 2015 10:45:41 +0100
Markus Armbruster arm...@redhat.com wrote:
This patch makes Coverity unhappy:
*** CID 1264326: Unintended sign extension (SIGN_EXTENSION)
/hw/s390x/s390-pci-inst.c: 787 in stpcifc_service_call()
781 stq_p(fib.pal, pbdev-pal);
782 stq_p(fib.iota, pbdev-g_iota);
783 stq_p(fib.aibv, pbdev-routes.adapter.ind_addr);
784 stq_p(fib.aisb, pbdev-routes.adapter.summary_addr);
785 stq_p(fib.fmb_addr, pbdev-fmb_addr);
786
CID 1264326: Unintended sign extension (SIGN_EXTENSION)
Suspicious implicit sign extension: pbdev-isc with type
unsigned char (8 bits, unsigned) is promoted in (pbdev-isc
28) | (pbdev-noi 16) to type int (32 bits, signed), then
sign-extended to type unsigned long (64 bits, unsigned). If
(pbdev-isc 28) | (pbdev-noi 16) is greater than
0x7FFF, the upper bits of the result will all be 1.
787 data = (pbdev-isc 28) | (pbdev-noi 16) |
788 (pbdev-routes.adapter.ind_offset 8) | (pbdev-sum 7) |
789pbdev-routes.adapter.summary_offset;
790 stw_p(fib.data, data);
791
792 if (pbdev-fh ENABLE_BIT_OFFSET) {
There's a fix for this (and the memory leak):
http://marc.info/?l=qemu-develm=142124886620078w=2
The patch is sitting in my queue, will send with the next pile of s390x
updates.
I can't see how
@@ -787,7 +787,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t
fiba)
data = (pbdev-isc 28) | (pbdev-noi 16) |
(pbdev-routes.adapter.ind_offset 8) | (pbdev-sum 7) |
pbdev-routes.adapter.summary_offset;
-stw_p(fib.data, data);
+stl_p(fib.data, data);
if (pbdev-fh ENABLE_BIT_OFFSET) {
fib.fc |= 0x80;
fixes the implicit sign extension within the assignment preceding it.
Let me explain it again real slow:
1. pbdev-isc gets promoted from uint8_t to int as operand of binary
(usual arithmetic conversions ISO/IEC 9899:1999 6.3.1.8)
2. The int result is shifted left 28 bits. This can set the MSB.
3. Likewise: pbdev-noi gets promoted from uint64_t to int, and shifted
left 16 bits.
4. The two shift results stay int and get ored.
5. pbdev-routes.adapter.ind_offset stays uint64_t, and is shifted left
8 bits.
6. The next or's left operand is the int result of 4 and the right
operant is the uint64_t result of 5. Therefore, the left operand is
*sign-extended* from int to uint64_t. This copies bit#7 of
pbdev-isc to bits#31..63. Whoops.
Regarding the leak, I prefer my patch, because it avoids the free on
error. But you're the maintainer.
Re: [Qemu-devel] [RESEND PATCH v1 0/5] Common unplug and unplug request cb for memory and CPU hot-unplug.
On Mon, 19 Jan 2015 23:29:37 +0200 Michael S. Tsirkin m...@redhat.com wrote: On Wed, Jan 07, 2015 at 02:49:40PM +0800, Tang Chen wrote: Memory and CPU hot unplug are both asynchronize procedures. When the unplug operation happens, unplug request cb is called first. And when ghest OS finished handling unplug, unplug cb will be called to do the real removal of device. They both need pc-machine, piix4 and ich9 unplug and unplug request cb. So this patch set introduces these commom functions as part1, and memory and CPU hot-unplug will come soon as part 2 and 3. This patch-set is based on QEmu 2.2 OK, Igor - you only have comments for the commit log? I take this as implicit ack of the patches? If so pls let me know. Yes, patches themselves are good. Can fixup commit messages and add author SoBs to them, before applying or should we wait for respin? Tang Chen (5): acpi, pc: Add hotunplug request cb for pc machine. acpi, ich9: Add hotunplug request cb for ich9. acpi, pc: Add unplug cb for pc machine. acpi, ich9: Add unplug cb for ich9. acpi, piix4: Add unplug cb for piix4. hw/acpi/ich9.c | 14 ++ hw/acpi/piix4.c| 8 hw/i386/pc.c | 16 hw/isa/lpc_ich9.c | 14 -- include/hw/acpi/ich9.h | 4 5 files changed, 54 insertions(+), 2 deletions(-) -- 1.8.4.2
Re: [Qemu-devel] [PATCH RFC v6 08/20] dataplane: allow virtio-1 devices
On Thu, Dec 11, 2014 at 02:25:10PM +0100, Cornelia Huck wrote: Handle endianness conversion for virtio-1 virtqueues correctly. Note that dataplane now needs to be built per-target. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/block/dataplane/virtio-blk.c |4 +- hw/scsi/virtio-scsi-dataplane.c |2 +- hw/virtio/Makefile.objs |2 +- hw/virtio/dataplane/Makefile.objs |2 +- hw/virtio/dataplane/vring.c | 86 ++--- include/hw/virtio/dataplane/vring-accessors.h | 75 + include/hw/virtio/dataplane/vring.h | 14 +--- 7 files changed, 131 insertions(+), 54 deletions(-) create mode 100644 include/hw/virtio/dataplane/vring-accessors.h This patch is independent of VIRTIO 1.0 and can be merged separately (faster). diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 1222a37..2d8cc15 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -16,7 +16,9 @@ #include qemu/iov.h #include qemu/thread.h #include qemu/error-report.h +#include hw/virtio/virtio-access.h #include hw/virtio/dataplane/vring.h +#include hw/virtio/dataplane/vring-accessors.h I like your vring-accessors.h approach better than the inline virtio_ld/st_p() in my patch. Nice. @@ -154,15 +157,18 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) } -static int get_desc(Vring *vring, VirtQueueElement *elem, +static int get_desc(VirtIODevice *vdev, Vring *vring, VirtQueueElement *elem, struct vring_desc *desc) Since we copy in struct vring_desc anyway, it's cleaner to byteswap the fields once instead of remembering to do it each time we need to access a field. The copy_in_vring_desc() function is one thing I prefer I about my patch. pgpvD7bFhoRlC.pgp Description: PGP signature
Re: [Qemu-devel] [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver
On Tue, 20 Jan 2015, Xu, Quan wrote:
-Original Message-
From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
Sent: Tuesday, January 20, 2015 1:15 AM
To: Xu, Quan
Cc: qemu-devel@nongnu.org; xen-de...@lists.xen.org;
stefano.stabell...@eu.citrix.com
Subject: Re: [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend
driver
On Tue, 30 Dec 2014, Quan Xu wrote:
This drvier transfers any request/repond between TPM xenstubdoms
driver and Xen vTPM stubdom, and facilitates communications between
Xen vTPM stubdom domain and vTPM xenstubdoms driver. It is a glue for
the TPM xenstubdoms driver and Xen stubdom vTPM domain that provides
the actual TPM functionality.
(Xen) Xen backend driver should run before running this frontend, and
initialize XenStore as the following for communication.
[XenStore]
..
FE.DOMAIN.ID
device =
vtpm =
0 =
backend =
/local/domain/{BE.DOMAIN.ID}/backend/vtpm/{FE.DOMAIN.ID}/0
backend-id = BE.DOMAIN.ID
state = 1
handle = 0
..
(QEMU) xen_vtpmdev_ops is initialized with the following process:
xen_hvm_init()
[...]
--xen_fe_register(vtpm, ...)
--xenstore_fe_scan()
--xen_fe_get_xendev()
-- XenDevOps.alloc()
--xen_fe_check()
-- XenDevOps.init()
-- XenDevOps.initialise()
-- XenDevOps.connected()
--xs_watch()
[...]
--Changes in v3:
-Move xen_stubdom_vtpm.c to xen_vtpm_frontend.c -Read Xen vTPM status
via XenStore
Signed-off-by: Quan Xu quan...@intel.com
---
hw/tpm/Makefile.objs | 1 +
hw/tpm/xen_vtpm_frontend.c | 264
+++
hw/xen/xen_backend.c | 34 ++
include/hw/xen/xen_backend.h | 9 +-
include/hw/xen/xen_common.h | 6 +
xen-hvm.c| 16 +++
6 files changed, 328 insertions(+), 2 deletions(-) create mode
100644 hw/tpm/xen_vtpm_frontend.c
diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs index
99f5983..57919fa 100644
--- a/hw/tpm/Makefile.objs
+++ b/hw/tpm/Makefile.objs
@@ -1,2 +1,3 @@
common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
+common-obj-$(CONFIG_TPM_XENSTUBDOMS) += xen_vtpm_frontend.o
diff --git a/hw/tpm/xen_vtpm_frontend.c b/hw/tpm/xen_vtpm_frontend.c
new file mode 100644 index 000..00cc888
--- /dev/null
+++ b/hw/tpm/xen_vtpm_frontend.c
@@ -0,0 +1,264 @@
+/*
+ * Connect to Xen vTPM stubdom domain
+ *
+ * Copyright (c) 2014 Intel Corporation
+ * Authors:
+ *Quan Xu quan...@intel.com
+ *
+ * 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 library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see
+http://www.gnu.org/licenses/ */
+
+#include stdio.h
+#include stdlib.h
+#include stdarg.h
+#include string.h
+#include unistd.h
+#include signal.h
+#include inttypes.h
+#include time.h
+#include fcntl.h
+#include errno.h
+#include sys/ioctl.h
+#include sys/types.h
+#include sys/stat.h
+#include sys/mman.h
+#include sys/uio.h
+
+#include hw/hw.h
+#include block/aio.h
+#include hw/xen/xen_backend.h
+
+enum tpmif_state {
+TPMIF_STATE_IDLE,/* no contents / vTPM idle / cancel
complete */
+TPMIF_STATE_SUBMIT, /* request ready / vTPM working */
+TPMIF_STATE_FINISH, /* response ready / vTPM idle */
+TPMIF_STATE_CANCEL, /* cancel requested / vTPM working */
+};
+
+static AioContext *vtpm_aio_ctx;
+
+enum status_bits {
+VTPM_STATUS_RUNNING = 0x1,
+VTPM_STATUS_IDLE = 0x2,
+VTPM_STATUS_RESULT = 0x4,
+VTPM_STATUS_CANCELED = 0x8,
+};
+
+struct tpmif_shared_page {
+uint32_t length; /* request/response length in bytes */
+
+uint8_t state; /* enum tpmif_state */
+uint8_t locality;/* for the current request */
+uint8_t pad; /* should be zero */
+
+uint8_t nr_extra_pages; /* extra pages for long packets; may be
zero
*/
+uint32_t extra_pages[0]; /* grant IDs; length is actually
+nr_extra_pages */ };
+
+struct xen_vtpm_dev {
+struct
Re: [Qemu-devel] [PATCH RFC v6 09/20] s390x/css: Add a callback for when subchannel gets disabled
On Thu, Dec 11, 2014 at 02:25:11PM +0100, Cornelia Huck wrote: From: Thomas Huth th...@linux.vnet.ibm.com We need a possibility to run code when a subchannel gets disabled. This patch adds the necessary infrastructure. Signed-off-by: Thomas Huth th...@linux.vnet.ibm.com Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/css.c | 12 hw/s390x/css.h |1 + 2 files changed, 13 insertions(+) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgp3ie_U39MCx.pgp Description: PGP signature
[Qemu-devel] [PULL 4/4] xen: add a lock for the mapcache
From: Paolo Bonzini pbonz...@redhat.com
Extend the existing dummy mapcache_lock/unlock macros to cover all of
xen-mapcache.c. This prepares for unlocked memory access, when parts
of exec.c will not be protected by the BQL.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
xen-mapcache.c | 54 +++---
1 file changed, 43 insertions(+), 11 deletions(-)
diff --git a/xen-mapcache.c b/xen-mapcache.c
index 458069b..8cefd0c 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -49,9 +49,6 @@
*/
#define NON_MCACHE_MEMORY_SIZE (80 * 1024 * 1024)
-#define mapcache_lock() ((void)0)
-#define mapcache_unlock() ((void)0)
-
typedef struct MapCacheEntry {
hwaddr paddr_index;
uint8_t *vaddr_base;
@@ -79,11 +76,22 @@ typedef struct MapCache {
unsigned int mcache_bucket_shift;
phys_offset_to_gaddr_t phys_offset_to_gaddr;
+QemuMutex lock;
void *opaque;
} MapCache;
static MapCache *mapcache;
+static inline void mapcache_lock(void)
+{
+qemu_mutex_lock(mapcache-lock);
+}
+
+static inline void mapcache_unlock(void)
+{
+qemu_mutex_unlock(mapcache-lock);
+}
+
static inline int test_bits(int nr, int size, const unsigned long *addr)
{
unsigned long res = find_next_zero_bit(addr, size + nr, nr);
@@ -102,6 +110,7 @@ void xen_map_cache_init(phys_offset_to_gaddr_t f, void
*opaque)
mapcache-phys_offset_to_gaddr = f;
mapcache-opaque = opaque;
+qemu_mutex_init(mapcache-lock);
QTAILQ_INIT(mapcache-locked_entries);
@@ -193,8 +202,8 @@ static void xen_remap_bucket(MapCacheEntry *entry,
g_free(err);
}
-uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size,
- uint8_t lock)
+static uint8_t *xen_map_cache_unlocked(hwaddr phys_addr, hwaddr size,
+ uint8_t lock)
{
MapCacheEntry *entry, *pentry = NULL;
hwaddr address_index;
@@ -291,14 +300,27 @@ tryagain:
return mapcache-last_entry-vaddr_base + address_offset;
}
+uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size,
+ uint8_t lock)
+{
+uint8_t *p;
+
+mapcache_lock();
+p = xen_map_cache_unlocked(phys_addr, size, lock);
+mapcache_unlock();
+return p;
+}
+
ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
{
MapCacheEntry *entry = NULL;
MapCacheRev *reventry;
hwaddr paddr_index;
hwaddr size;
+ram_addr_t raddr;
int found = 0;
+mapcache_lock();
QTAILQ_FOREACH(reventry, mapcache-locked_entries, next) {
if (reventry-vaddr_req == ptr) {
paddr_index = reventry-paddr_index;
@@ -323,13 +345,16 @@ ram_addr_t xen_ram_addr_from_mapcache(void *ptr)
}
if (!entry) {
DPRINTF(Trying to find address %p that is not in the mapcache!\n,
ptr);
-return 0;
+raddr = 0;
+} else {
+raddr = (reventry-paddr_index MCACHE_BUCKET_SHIFT) +
+ ((unsigned long) ptr - (unsigned long) entry-vaddr_base);
}
-return (reventry-paddr_index MCACHE_BUCKET_SHIFT) +
-((unsigned long) ptr - (unsigned long) entry-vaddr_base);
+mapcache_unlock();
+return raddr;
}
-void xen_invalidate_map_cache_entry(uint8_t *buffer)
+static void xen_invalidate_map_cache_entry_unlocked(uint8_t *buffer)
{
MapCacheEntry *entry = NULL, *pentry = NULL;
MapCacheRev *reventry;
@@ -383,6 +408,13 @@ void xen_invalidate_map_cache_entry(uint8_t *buffer)
g_free(entry);
}
+void xen_invalidate_map_cache_entry(uint8_t *buffer)
+{
+mapcache_lock();
+xen_invalidate_map_cache_entry_unlocked(buffer);
+mapcache_unlock();
+}
+
void xen_invalidate_map_cache(void)
{
unsigned long i;
@@ -391,14 +423,14 @@ void xen_invalidate_map_cache(void)
/* Flush pending AIO before destroying the mapcache */
bdrv_drain_all();
+mapcache_lock();
+
QTAILQ_FOREACH(reventry, mapcache-locked_entries, next) {
DPRINTF(There should be no locked mappings at this time,
but TARGET_FMT_plx - %p is present\n,
reventry-paddr_index, reventry-vaddr_req);
}
-mapcache_lock();
-
for (i = 0; i mapcache-nr_buckets; i++) {
MapCacheEntry *entry = mapcache-entry[i];
--
1.7.10.4
Re: [Qemu-devel] [PULL 0/4] Xen tree 2015-01-20
On 20 January 2015 at 11:19, Stefano Stabellini stefano.stabell...@eu.citrix.com wrote: The following changes since commit 74acb99737dbedd86654d660c0c20815139a873c: Merge remote-tracking branch 'remotes/kraxel/tags/pull-console-20150119-1' into staging (2015-01-19 13:37:05 +) are available in the git repository at: git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-2015-01-20 for you to fetch changes up to 085bde8e8f9bd4fb06e010810991b26aba795fb2: xen: add a lock for the mapcache (2015-01-20 11:09:54 +) I'm afraid I can't apply this -- three out of four patches are missing your signed-off-by as maintainer. You might like to add something to your pre-submission checking scripts to catch this error; eg the script fragment I suggested in http://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg02134.html thanks -- PMM
[Qemu-devel] [PATCH] s390: Plug memory leak on s390_pci_generate_event() error path
Signed-off-by: Markus Armbruster arm...@redhat.com
---
hw/s390x/s390-pci-bus.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 1201b8d..d25ac74 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -187,7 +187,7 @@ S390PCIBusDevice *s390_pci_find_dev_by_fh(uint32_t fh)
static void s390_pci_generate_event(uint8_t cc, uint16_t pec, uint32_t fh,
uint32_t fid, uint64_t faddr, uint32_t e)
{
-SeiContainer *sei_cont = g_malloc0(sizeof(SeiContainer));
+SeiContainer *sei_cont;
S390pciState *s = S390_PCI_HOST_BRIDGE(
object_resolve_path(TYPE_S390_PCI_HOST_BRIDGE, NULL));
@@ -195,6 +195,7 @@ static void s390_pci_generate_event(uint8_t cc, uint16_t
pec, uint32_t fh,
return;
}
+sei_cont = g_malloc0(sizeof(SeiContainer));
sei_cont-fh = fh;
sei_cont-fid = fid;
sei_cont-cc = cc;
--
1.9.3
Re: [Qemu-devel] [v3 2/5] Qemu-Xen-vTPM: Xen frontend driver infrastructure
On Tue, 20 Jan 2015, Xu, Quan wrote:
-Original Message-
From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
Sent: Tuesday, January 20, 2015 1:15 AM
To: Xu, Quan
Cc: qemu-devel@nongnu.org; xen-de...@lists.xen.org;
stefano.stabell...@eu.citrix.com
Subject: Re: [v3 2/5] Qemu-Xen-vTPM: Xen frontend driver infrastructure
On Tue, 30 Dec 2014, Quan Xu wrote:
This patch adds infrastructure for xen front drivers living in qemu,
so drivers don't need to implement common stuff on their own. It's
mostly xenbus management stuff: some functions to access XenStore,
setting up XenStore watches, callbacks on device discovery and state
changes, and handle event channel between the virtual machines.
Call xen_fe_register() function to register XenDevOps, and make sure,
XenDevOps's flags is DEVOPS_FLAG_FE, which is flag bit to point out
the XenDevOps is Xen frontend.
--Changes in v3:
-New xen_frontend.c file
-Move xenbus_switch_state() to xen_frontend.c -Move xen_stubdom_be()
to xenstore_fe_read_be_str() -Move *_stubdom_*() to *_fe_*()
Signed-off-by: Quan Xu quan...@intel.com
---
hw/xen/Makefile.objs | 2 +-
hw/xen/xen_backend.c | 11 +-
hw/xen/xen_frontend.c| 323
+++
include/hw/xen/xen_backend.h | 14 ++
4 files changed, 348 insertions(+), 2 deletions(-) create mode
100644 hw/xen/xen_frontend.c
diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs index
a0ca0aa..b0bb065 100644
--- a/hw/xen/Makefile.objs
+++ b/hw/xen/Makefile.objs
@@ -1,5 +1,5 @@
# xen backend driver support
-common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
+common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o xen_devconfig.o
+xen_frontend.o
obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o
xen_pt_msi.o diff --git a/hw/xen/xen_backend.c b/hw/xen/xen_backend.c
index b2cb22b..ad6e324 100644
--- a/hw/xen/xen_backend.c
+++ b/hw/xen/xen_backend.c
@@ -275,7 +275,7 @@ static struct XenDevice *xen_be_get_xendev(const
char *type, int dom, int dev,
/*
* release xen backend device.
*/
-static struct XenDevice *xen_be_del_xendev(int dom, int dev)
+struct XenDevice *xen_be_del_xendev(int dom, int dev)
{
struct XenDevice *xendev, *xnext;
@@ -681,6 +681,10 @@ static void xenstore_update(void *unused)
if (sscanf(vec[XS_WATCH_TOKEN], fe:% PRIxPTR, ptr) == 1) {
xenstore_update_fe(vec[XS_WATCH_PATH], (void*)ptr);
}
+if (sscanf(vec[XS_WATCH_TOKEN], stub:% PRIxPTR :%d:% PRIxPTR,
+ type, dom, ops) == 3) {
+xenstore_fe_update(vec[XS_WATCH_PATH], (void *)type, dom,
(void *)ops);
+}
Why stub:? Where is it coming from?
'stub:' is a head of token for xs_watch. The better head is 'fe:', but it is
used by backend.
So I call it 'stub:', which refers to 'Xen stubdomain backend'.
'stub:' is initialized in xenstore_fe_scan().
Ah, right! In that case, please just use be:% because here we don't
care where the backend is (stubdom, dom0, etc), we only care that is a
backend.
I think that the xenstore_update function should be moved to a new file:
we don't want xen_backend.c to be handling any frontend updates.
Maybe you could create a new file, hw/xen/xen_pvdev.c?
Make sense.
cleanup:
free(vec);
@@ -808,3 +812,8 @@ void xen_be_printf(struct XenDevice *xendev, int
msg_level, const char *fmt, ...
}
qemu_log_flush();
}
+
+void xen_qtail_insert_xendev(struct XenDevice *xendev) {
+QTAILQ_INSERT_TAIL(xendevs, xendev, next); }
You should either move the xendevs queue to the new file xen_pvdev.c or you
should introduce a separate xendevs queue for frontends in xen_frontend.c.
diff --git a/hw/xen/xen_frontend.c b/hw/xen/xen_frontend.c new file
mode 100644 index 000..07ffc5c
--- /dev/null
+++ b/hw/xen/xen_frontend.c
@@ -0,0 +1,323 @@
+/*
+ * Xen frontend driver infrastructure
+ *
+ * Copyright (c) 2014 Intel Corporation
+ * Authors:
+ *Quan Xu quan...@intel.com
+ *
+ * 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 library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if
Re: [Qemu-devel] [PATCH RFC v6 15/20] virtio-net: no writeable mac for virtio-1
On Thu, Dec 11, 2014 at 02:25:17PM +0100, Cornelia Huck wrote: Devices operating as virtio 1.0 may not allow writes to the mac address in config space. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/net/virtio-net.c |1 + 1 file changed, 1 insertion(+) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index d6d1b98..ebbea60 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -87,6 +87,7 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config) memcpy(netcfg, config, n-config_size); if (!virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR) I don't see VIRTIO_NET_F_CTRL_MAC_ADDR (23) in the VIRTIO 1.0 5.1.3.1 Legacy Interface: Feature bits section. Should it be there just so people don't try to reuse bit 23 in the future? The patch itself: Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgp38LhNAENrH.pgp Description: PGP signature
[Qemu-devel] [PATCH] target-mips: Clean up switch fall through after commit fecd264
Commit fecd264 added a number of fall-throughs, but neglected to properly document them as intentional. Commit d922445 cleaned that up for many, but not all cases. Take care of the remaining ones. Spotted by Coverity. Signed-off-by: Markus Armbruster arm...@redhat.com --- target-mips/translate.c | 4 1 file changed, 4 insertions(+) diff --git a/target-mips/translate.c b/target-mips/translate.c index e9d86b2..8abc12b 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -18729,6 +18729,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) case OPC_SWL: case OPC_SWR: check_insn_opc_removed(ctx, ISA_MIPS32R6); + /* fall through */ case OPC_SB ... OPC_SH: case OPC_SW: gen_st(ctx, op, rt, rs, imm); @@ -18817,6 +18818,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) case OPC_PS_FMT: check_cp1_enabled(ctx); check_insn_opc_removed(ctx, ISA_MIPS32R6); +/* fall through */ case OPC_S_FMT: case OPC_D_FMT: check_cp1_enabled(ctx); @@ -19000,6 +19002,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) case OPC_LDL ... OPC_LDR: case OPC_LLD: check_insn_opc_removed(ctx, ISA_MIPS32R6); +/* fall through */ case OPC_LWU: case OPC_LD: check_insn(ctx, ISA_MIPS3); @@ -19008,6 +19011,7 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx) break; case OPC_SDL ... OPC_SDR: check_insn_opc_removed(ctx, ISA_MIPS32R6); +/* fall through */ case OPC_SD: check_insn(ctx, ISA_MIPS3); check_mips_64(ctx); -- 1.9.3
Re: [Qemu-devel] [PATCH 0/2] m48t59: add year offset and MMIO ISA mapping
On Mon, Jan 19, 2015 at 10:59 PM, Hervé Poussineau hpous...@reactos.org wrote: Le 19/01/2015 12:35, Mark Cave-Ayland a écrit : This patch lays the groundwork for switching sun4u over from ioport NVRAM access to MMIO NVRAM access. Patch 1 introduces a new year_offset property which is the offset between the year value stored in hardware and the actual year. In particular, Sun hardware has a 68 year offset used to extend the date range of the IC. While the kernel sources I have viewed contain this offset within a #ifdef CONFIG_SPARC block, I've updated all the callers so that no change in behaviour will be seen across all platforms. PPC users may wish to alter the callers to change this behaviour as required. Patch 2 mimics the mem_base parameter from m48t59_init() to m48t59_init_isa() so that MMIO can be used for sun4u where the NVRAM is attached to the ebus which is essentially the same as an ISA bus. I've a patch which QOM'ifies m48t59, that I'll send to the list. If you apply it, you'll be then able to create a sysbus-m48t02 device, and then to add it to ebus memory region. Why m48t02? As discussed before, it shall be a m48t59 device: http://lists.gnu.org/archive/html/qemu-devel/2013-04/msg05559.html Artyom -- Regards, Artyom Tarasenko SPARC and PPC PReP under qemu blog: http://tyom.blogspot.com/search/label/qemu
Re: [Qemu-devel] [PATCH v0 2/2] pc-dimm: Make pc_existing_dimms_capacity global
On Mon, 12 Jan 2015 09:32:34 +0530
Bharata B Rao bhar...@linux.vnet.ibm.com wrote:
Move pc_existing_dimms_capacity() to pc-dimm.c since it would be needed
by PowerPC memory hotplug code too.
Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com
---
hw/i386/pc.c | 24
hw/mem/pc-dimm.c | 25 +
include/hw/mem/pc-dimm.h | 1 +
3 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 125cf0a..2ec45a4 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1552,30 +1552,6 @@ void qemu_register_pc_machine(QEMUMachine *m)
g_free(name);
}
-static int pc_existing_dimms_capacity(Object *obj, void *opaque)
-{
-Error *local_err = NULL;
-uint64_t *size = opaque;
-
-if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
-DeviceState *dev = DEVICE(obj);
-
-if (dev-realized) {
-(*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
-local_err);
-}
-
-if (local_err) {
-qerror_report_err(local_err);
-error_free(local_err);
-return 1;
-}
-}
-
-object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
-return 0;
-}
-
static void pc_dimm_plug(HotplugHandler *hotplug_dev,
DeviceState *dev, Error **errp)
{
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index d431834..f02ce6e 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -22,6 +22,31 @@
#include qemu/config-file.h
#include qapi/visitor.h
#include qemu/range.h
+#include qapi/qmp/qerror.h
+
+int pc_existing_dimms_capacity(Object *obj, void *opaque)
+{
since you are making it API, could you pass Error **errp argument
and deal with error in caller?
Along with it you can make function return void.
+Error *local_err = NULL;
+uint64_t *size = opaque;
+
+if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
+DeviceState *dev = DEVICE(obj);
+
+if (dev-realized) {
+(*size) += object_property_get_int(obj, PC_DIMM_SIZE_PROP,
+local_err);
+}
+
+if (local_err) {
+qerror_report_err(local_err);
+error_free(local_err);
+return 1;
+}
+}
+
+object_child_foreach(obj, pc_existing_dimms_capacity, opaque);
+return 0;
+}
int qmp_pc_dimm_device_list(Object *obj, void *opaque)
{
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index e1dcbbc..bbfa53f 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -78,4 +78,5 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
int pc_dimm_get_free_slot(const int *hint, int max_slots, Error **errp);
int qmp_pc_dimm_device_list(Object *obj, void *opaque);
+int pc_existing_dimms_capacity(Object *obj, void *opaque);
#endif
Re: [Qemu-devel] [PATCH v2 1/4] pc: append ssdt-misc.dsl to the DSDT
On 20/01/2015 10:59, Igor Mammedov wrote: Yes, trimming is better than putting it in the DSDT, at least for simple devices such as SMC and pvpanic. So are we dropping 1-2/4 from this series? I need to know on top of what to rebase. I'll take care of moving SMC to SSDT. Do not rebase on anything. You go first. Paolo
Re: [Qemu-devel] [PATCH v3 0/3] move ssdt-misc.dsl to the DSDT
On Tue, 20 Jan 2015 08:05:04 +0100 Paolo Bonzini pbonz...@redhat.com wrote: On 19/01/2015 22:31, Michael S. Tsirkin wrote: On Mon, Jan 19, 2015 at 05:56:24PM +0100, Paolo Bonzini wrote: See v2 for motivation. v2-v3: dropped pointer passing and patch 4. Thanks! Igor - ok with you? FWIW I'm okay with moving stuff back to the SSDT as part of Igor's other ACPI series. The #included files would remain in acpi-dsdt-common.dsl. can we drop 1-2 patches and not move stuff uselessly back and forth? (it only complicates rebase of big series and nothing else) I'll take care of SMC device and move it into SSDT with respin of AML API series. And taking in account that ROMs are resizable now, I can trim not present SSDT devices along with it. Paolo
Re: [Qemu-devel] [PATCH RFC v6 10/20] s390x/virtio-ccw: add virtio set-revision call
On Thu, Dec 11, 2014 at 02:25:12PM +0100, Cornelia Huck wrote:
@@ -608,6 +631,25 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
}
}
break;
+case CCW_CMD_SET_VIRTIO_REV:
+len = sizeof(revinfo);
+if (ccw.count len || (check_len ccw.count len)) {
+ret = -EINVAL;
+break;
+}
+if (!ccw.cda) {
+ret = -EFAULT;
+break;
+}
+cpu_physical_memory_read(ccw.cda, revinfo, len);
+if (dev-revision = 0 ||
+revinfo.revision virtio_ccw_rev_max(dev)) {
In the next patch virtio_ccw_handle_set_vq() uses big-endian memory
access functions to load a struct from guest memory.
Here you just copy the struct in without byteswaps.
Are the byteswaps missing here? (I guess this normally runs big-endian
guests on big-endian hosts so it's not noticable.)
Stefan
pgplXEs9ODsoc.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH RFC v6 14/20] s390x/virtio-ccw: enable virtio 1.0
On Thu, Dec 11, 2014 at 02:25:16PM +0100, Cornelia Huck wrote: virtio-ccw should now have everything in place to operate virtio 1.0 devices, so let's enable revision 1. Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com --- hw/s390x/virtio-ccw.h |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi stefa...@redhat.com pgpISOu_zMZVC.pgp Description: PGP signature
[Qemu-devel] [PULL 3/4] xen: do not use __-named variables in mapcache
From: Paolo Bonzini pbonz...@redhat.com
Keep the namespace clean.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
xen-mapcache.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/xen-mapcache.c b/xen-mapcache.c
index 66da1a6..458069b 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -199,8 +199,8 @@ uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size,
MapCacheEntry *entry, *pentry = NULL;
hwaddr address_index;
hwaddr address_offset;
-hwaddr __size = size;
-hwaddr __test_bit_size = size;
+hwaddr cache_size = size;
+hwaddr test_bit_size;
bool translated = false;
tryagain:
@@ -209,22 +209,22 @@ tryagain:
trace_xen_map_cache(phys_addr);
-/* __test_bit_size is always a multiple of XC_PAGE_SIZE */
+/* test_bit_size is always a multiple of XC_PAGE_SIZE */
if (size) {
-__test_bit_size = size + (phys_addr (XC_PAGE_SIZE - 1));
+test_bit_size = size + (phys_addr (XC_PAGE_SIZE - 1));
-if (__test_bit_size % XC_PAGE_SIZE) {
-__test_bit_size += XC_PAGE_SIZE - (__test_bit_size % XC_PAGE_SIZE);
+if (test_bit_size % XC_PAGE_SIZE) {
+test_bit_size += XC_PAGE_SIZE - (test_bit_size % XC_PAGE_SIZE);
}
} else {
-__test_bit_size = XC_PAGE_SIZE;
+test_bit_size = XC_PAGE_SIZE;
}
if (mapcache-last_entry != NULL
mapcache-last_entry-paddr_index == address_index
-!lock !__size
+!lock !size
test_bits(address_offset XC_PAGE_SHIFT,
- __test_bit_size XC_PAGE_SHIFT,
+ test_bit_size XC_PAGE_SHIFT,
mapcache-last_entry-valid_mapping)) {
trace_xen_map_cache_return(mapcache-last_entry-vaddr_base +
address_offset);
return mapcache-last_entry-vaddr_base + address_offset;
@@ -232,20 +232,20 @@ tryagain:
/* size is always a multiple of MCACHE_BUCKET_SIZE */
if (size) {
-__size = size + address_offset;
-if (__size % MCACHE_BUCKET_SIZE) {
-__size += MCACHE_BUCKET_SIZE - (__size % MCACHE_BUCKET_SIZE);
+cache_size = size + address_offset;
+if (cache_size % MCACHE_BUCKET_SIZE) {
+cache_size += MCACHE_BUCKET_SIZE - (cache_size %
MCACHE_BUCKET_SIZE);
}
} else {
-__size = MCACHE_BUCKET_SIZE;
+cache_size = MCACHE_BUCKET_SIZE;
}
entry = mapcache-entry[address_index % mapcache-nr_buckets];
while (entry entry-lock entry-vaddr_base
-(entry-paddr_index != address_index || entry-size != __size ||
+(entry-paddr_index != address_index || entry-size != cache_size
||
!test_bits(address_offset XC_PAGE_SHIFT,
- __test_bit_size XC_PAGE_SHIFT,
+ test_bit_size XC_PAGE_SHIFT,
entry-valid_mapping))) {
pentry = entry;
entry = entry-next;
@@ -253,19 +253,19 @@ tryagain:
if (!entry) {
entry = g_malloc0(sizeof (MapCacheEntry));
pentry-next = entry;
-xen_remap_bucket(entry, __size, address_index);
+xen_remap_bucket(entry, cache_size, address_index);
} else if (!entry-lock) {
if (!entry-vaddr_base || entry-paddr_index != address_index ||
-entry-size != __size ||
+entry-size != cache_size ||
!test_bits(address_offset XC_PAGE_SHIFT,
-__test_bit_size XC_PAGE_SHIFT,
+test_bit_size XC_PAGE_SHIFT,
entry-valid_mapping)) {
-xen_remap_bucket(entry, __size, address_index);
+xen_remap_bucket(entry, cache_size, address_index);
}
}
if(!test_bits(address_offset XC_PAGE_SHIFT,
-__test_bit_size XC_PAGE_SHIFT,
+test_bit_size XC_PAGE_SHIFT,
entry-valid_mapping)) {
mapcache-last_entry = NULL;
if (!translated mapcache-phys_offset_to_gaddr) {
--
1.7.10.4
Re: [Qemu-devel] [PATCH] apic: do not dereference pointer before it is checked for NULL
Paolo Bonzini pbonz...@redhat.com writes: Right now you only get to apic_init_reset if you have an APIC (do_cpu_init is reached only if CPU_INTERRUPT_INIT is set and that only happens in hw/intc/apic.c). However, this is wrong because for example a port 92 or keyboard controller reset is really an INIT, and that can happen also with no APIC. So keep the check and fix the error that Coverity reported. Reported-by: Markus Armbruster arm...@redhat.com Signed-off-by: Paolo Bonzini pbonz...@redhat.com Reviewed-by: Markus Armbruster arm...@redhat.com
Re: [Qemu-devel] [RFC PATCH v3] tests: rtl8139: test timers and interrupt
On 08/01/2015 19:38, Frediano Ziglio wrote:
Test behaviour of timers and interrupts related to timeouts.
Signed-off-by: Frediano Ziglio fredd...@gmail.com
---
tests/Makefile | 2 +-
tests/rtl8139-test.c | 181
+++
2 files changed, 182 insertions(+), 1 deletion(-)
This patch was derived from a test I did while implementing timer in
rtl8139 code. Now that there is support for integrated testing I converted
it. The test was tested on a real NIC.
As if it's the first test I wrote I don't know if syntax and details are
fine. For instance should I remove nop test? Should I split my test?
Respectively, no and if you want.
Reviewed-by: Paolo Bonzini pbonz...@redhat.com
As the last person who touched the rtl8139 timer code, I'm glad I didn't
break anything. :)
Paolo
Changed from v2:
- style (variable declaration, Perl script not able to spot it)
Changed from v1:
- style
diff --git a/tests/Makefile b/tests/Makefile
index e4ddb6a..8858407 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -320,7 +320,7 @@ tests/tmp105-test$(EXESUF): tests/tmp105-test.o
$(libqos-omap-obj-y)
tests/i440fx-test$(EXESUF): tests/i440fx-test.o $(libqos-pc-obj-y)
tests/fw_cfg-test$(EXESUF): tests/fw_cfg-test.o $(libqos-pc-obj-y)
tests/e1000-test$(EXESUF): tests/e1000-test.o
-tests/rtl8139-test$(EXESUF): tests/rtl8139-test.o
+tests/rtl8139-test$(EXESUF): tests/rtl8139-test.o $(libqos-pc-obj-y)
tests/pcnet-test$(EXESUF): tests/pcnet-test.o
tests/eepro100-test$(EXESUF): tests/eepro100-test.o
tests/vmxnet3-test$(EXESUF): tests/vmxnet3-test.o
diff --git a/tests/rtl8139-test.c b/tests/rtl8139-test.c
index f6a1be3..4e0bf02 100644
--- a/tests/rtl8139-test.c
+++ b/tests/rtl8139-test.c
@@ -10,19 +10,200 @@
#include glib.h
#include string.h
#include libqtest.h
+#include libqos/pci-pc.h
#include qemu/osdep.h
+#include qemu-common.h
/* Tests only initialization so far. TODO: Replace with functional tests */
static void nop(void)
{
}
+#define CLK 3300
+#define NS_PER_SEC 10ULL
+
+static QPCIBus *pcibus;
+static QPCIDevice *dev;
+static void *dev_base;
+
+static void save_fn(QPCIDevice *dev, int devfn, void *data)
+{
+QPCIDevice **pdev = (QPCIDevice **) data;
+
+*pdev = dev;
+}
+
+static QPCIDevice *get_device(void)
+{
+QPCIDevice *dev;
+
+pcibus = qpci_init_pc();
+qpci_device_foreach(pcibus, 0x10ec, 0x8139, save_fn, dev);
+g_assert(dev != NULL);
+
+return dev;
+}
+
+#define PORT(name, len, val) \
+static unsigned __attribute__((unused)) in_##name(void) \
+{ \
+unsigned res = qpci_io_read##len(dev, dev_base+(val)); \
+g_test_message(*%s - %x\n, #name, res); \
+return res; \
+} \
+static void out_##name(unsigned v) \
+{ \
+g_test_message(%x - *%s\n, v, #name); \
+qpci_io_write##len(dev, dev_base+(val), v); \
+}
+
+PORT(Timer, l, 0x48)
+PORT(IntrMask, w, 0x3c)
+PORT(IntrStatus, w, 0x3E)
+PORT(TimerInt, l, 0x54)
+
+#define fatal(...) do { g_test_message(__VA_ARGS__); g_assert(0); } while (0)
+
+static void test_timer(void)
+{
+const unsigned from = 0.95 * CLK;
+const unsigned to = 1.6 * CLK;
+unsigned prev, curr, next;
+unsigned cnt, diff;
+
+out_IntrMask(0);
+
+in_IntrStatus();
+in_Timer();
+in_Timer();
+
+/* Test 1. test counter continue and continue */
+out_TimerInt(0); /* disable timer */
+out_IntrStatus(0x4000);
+out_Timer(12345); /* reset timer to 0 */
+curr = in_Timer();
+if (curr 0.1 * CLK) {
+fatal(time too big %u\n, curr);
+}
+for (cnt = 0; ; ) {
+clock_step(1 * NS_PER_SEC);
+prev = curr;
+curr = in_Timer();
+
+/* test skip is in a specific range */
+diff = (curr-prev) 0xu;
+if (diff from || diff to) {
+fatal(Invalid diff %u (%u-%u)\n, diff, from, to);
+}
+if (curr prev ++cnt == 3) {
+break;
+}
+}
+
+/* Test 2. Check we didn't get an interrupt with TimerInt == 0 */
+if (in_IntrStatus() 0x4000) {
+fatal(got an interrupt\n);
+}
+
+/* Test 3. Setting TimerInt to 1 and Timer to 0 get interrupt */
+out_TimerInt(1);
+out_Timer(0);
+clock_step(40);
+if ((in_IntrStatus() 0x4000) == 0) {
+fatal(we should have an interrupt here!\n);
+}
+
+/* Test 3. Check acknowledge */
+out_IntrStatus(0x4000);
+if (in_IntrStatus() 0x4000) {
+fatal(got an interrupt\n);
+}
+
+/* Test. Status set after Timer reset */
+out_Timer(0);
+out_TimerInt(0);
+out_IntrStatus(0x4000);
+curr = in_Timer();
+out_TimerInt(curr + 0.5 * CLK);
+clock_step(1 * NS_PER_SEC);
+out_Timer(0);
+if ((in_IntrStatus() 0x4000) == 0) {
+fatal(we should have an interrupt here!\n);
+}
+
+
[Qemu-devel] [PATCH] target-arm/translate-a64: Fix wrong mmu_idx usage for LDT/STT
The LDT/STT (load/store unprivileged) instruction decode was using
the wrong MMU index value. This meant that instead of these insns
being always access as if user-mode regardless of current privilege
they were always access as if kernel-mode regardless of current
privilege. This went unnoticed because AArch64 Linux doesn't use
these instructions.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
I'm not counting this as a security issue because I'm assuming
nobody treats TCG guests as a security boundary (certainly I
would not recommend doing so...)
---
target-arm/translate-a64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 80d2359..dac2f63 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -2107,7 +2107,7 @@ static void disas_ldst_reg_imm9(DisasContext *s, uint32_t
insn)
}
} else {
TCGv_i64 tcg_rt = cpu_reg(s, rt);
-int memidx = is_unpriv ? 1 : get_mem_index(s);
+int memidx = is_unpriv ? MMU_USER_IDX : get_mem_index(s);
if (is_store) {
do_gpr_st_memidx(s, tcg_rt, tcg_addr, size, memidx);
--
1.9.1
[Qemu-devel] [PULL v2 2/4] Xen: Use the ioreq-server API when available
From: Paul Durrant paul.durr...@citrix.com
The ioreq-server API added to Xen 4.5 offers better security than
the existing Xen/QEMU interface because the shared pages that are
used to pass emulation request/results back and forth are removed
from the guest's memory space before any requests are serviced.
This prevents the guest from mapping these pages (they are in a
well known location) and attempting to attack QEMU by synthesizing
its own request structures. Hence, this patch modifies configure
to detect whether the API is available, and adds the necessary
code to use the API if it is.
Signed-off-by: Paul Durrant paul.durr...@citrix.com
Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
Acked-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
---
configure | 29 ++
include/hw/xen/xen_common.h | 223 +++
trace-events|9 ++
xen-hvm.c | 160 ++-
4 files changed, 399 insertions(+), 22 deletions(-)
diff --git a/configure b/configure
index 7539645..5ea1014 100755
--- a/configure
+++ b/configure
@@ -1877,6 +1877,32 @@ int main(void) {
xc_gnttab_open(NULL, 0);
xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
+ xc_hvm_create_ioreq_server(xc, 0, 0, NULL);
+ return 0;
+}
+EOF
+ compile_prog $xen_libs
+then
+xen_ctrl_version=450
+xen=yes
+
+ elif
+ cat $TMPC EOF
+#include xenctrl.h
+#include xenstore.h
+#include stdint.h
+#include xen/hvm/hvm_info_table.h
+#if !defined(HVM_MAX_VCPUS)
+# error HVM_MAX_VCPUS not defined
+#endif
+int main(void) {
+ xc_interface *xc;
+ xs_daemon_open();
+ xc = xc_interface_open(0, 0, 0);
+ xc_hvm_set_mem_type(0, 0, HVMMEM_ram_ro, 0, 0);
+ xc_gnttab_open(NULL, 0);
+ xc_domain_add_to_physmap(0, 0, XENMAPSPACE_gmfn, 0, 0);
+ xc_hvm_inject_msi(xc, 0, 0xf000, 0x);
return 0;
}
EOF
@@ -4283,6 +4309,9 @@ if test -n $sparc_cpu; then
echo Target Sparc Arch $sparc_cpu
fi
echo xen support $xen
+if test $xen = yes ; then
+ echo xen ctrl version $xen_ctrl_version
+fi
echo brlapi support$brlapi
echo bluez support$bluez
echo Documentation $docs
diff --git a/include/hw/xen/xen_common.h b/include/hw/xen/xen_common.h
index 95612a4..519696f 100644
--- a/include/hw/xen/xen_common.h
+++ b/include/hw/xen/xen_common.h
@@ -16,7 +16,9 @@
#include hw/hw.h
#include hw/xen/xen.h
+#include hw/pci/pci.h
#include qemu/queue.h
+#include trace.h
/*
* We don't support Xen prior to 3.3.0.
@@ -179,4 +181,225 @@ static inline int xen_get_vmport_regs_pfn(XenXC xc,
domid_t dom,
}
#endif
+/* Xen before 4.5 */
+#if CONFIG_XEN_CTRL_INTERFACE_VERSION 450
+
+#ifndef HVM_PARAM_BUFIOREQ_EVTCHN
+#define HVM_PARAM_BUFIOREQ_EVTCHN 26
+#endif
+
+#define IOREQ_TYPE_PCI_CONFIG 2
+
+typedef uint32_t ioservid_t;
+
+static inline void xen_map_memory_section(XenXC xc, domid_t dom,
+ ioservid_t ioservid,
+ MemoryRegionSection *section)
+{
+}
+
+static inline void xen_unmap_memory_section(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+MemoryRegionSection *section)
+{
+}
+
+static inline void xen_map_io_section(XenXC xc, domid_t dom,
+ ioservid_t ioservid,
+ MemoryRegionSection *section)
+{
+}
+
+static inline void xen_unmap_io_section(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+MemoryRegionSection *section)
+{
+}
+
+static inline void xen_map_pcidev(XenXC xc, domid_t dom,
+ ioservid_t ioservid,
+ PCIDevice *pci_dev)
+{
+}
+
+static inline void xen_unmap_pcidev(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+PCIDevice *pci_dev)
+{
+}
+
+static inline int xen_create_ioreq_server(XenXC xc, domid_t dom,
+ ioservid_t *ioservid)
+{
+return 0;
+}
+
+static inline void xen_destroy_ioreq_server(XenXC xc, domid_t dom,
+ioservid_t ioservid)
+{
+}
+
+static inline int xen_get_ioreq_server_info(XenXC xc, domid_t dom,
+ioservid_t ioservid,
+xen_pfn_t *ioreq_pfn,
+xen_pfn_t *bufioreq_pfn,
+evtchn_port_t *bufioreq_evtchn)
+{
+unsigned long param;
+int rc;
+
+rc = xc_get_hvm_param(xc, dom, HVM_PARAM_IOREQ_PFN, param);
+if (rc 0) {
+fprintf(stderr, failed to get HVM_PARAM_IOREQ_PFN\n);
+return -1;
+
[Qemu-devel] [PULL v2 3/4] xen: do not use __-named variables in mapcache
From: Paolo Bonzini pbonz...@redhat.com
Keep the namespace clean.
Signed-off-by: Paolo Bonzini pbonz...@redhat.com
Signed-off-by: Stefano Stabellini stefano.stabell...@eu.citrix.com
---
xen-mapcache.c | 40
1 file changed, 20 insertions(+), 20 deletions(-)
diff --git a/xen-mapcache.c b/xen-mapcache.c
index 66da1a6..458069b 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -199,8 +199,8 @@ uint8_t *xen_map_cache(hwaddr phys_addr, hwaddr size,
MapCacheEntry *entry, *pentry = NULL;
hwaddr address_index;
hwaddr address_offset;
-hwaddr __size = size;
-hwaddr __test_bit_size = size;
+hwaddr cache_size = size;
+hwaddr test_bit_size;
bool translated = false;
tryagain:
@@ -209,22 +209,22 @@ tryagain:
trace_xen_map_cache(phys_addr);
-/* __test_bit_size is always a multiple of XC_PAGE_SIZE */
+/* test_bit_size is always a multiple of XC_PAGE_SIZE */
if (size) {
-__test_bit_size = size + (phys_addr (XC_PAGE_SIZE - 1));
+test_bit_size = size + (phys_addr (XC_PAGE_SIZE - 1));
-if (__test_bit_size % XC_PAGE_SIZE) {
-__test_bit_size += XC_PAGE_SIZE - (__test_bit_size % XC_PAGE_SIZE);
+if (test_bit_size % XC_PAGE_SIZE) {
+test_bit_size += XC_PAGE_SIZE - (test_bit_size % XC_PAGE_SIZE);
}
} else {
-__test_bit_size = XC_PAGE_SIZE;
+test_bit_size = XC_PAGE_SIZE;
}
if (mapcache-last_entry != NULL
mapcache-last_entry-paddr_index == address_index
-!lock !__size
+!lock !size
test_bits(address_offset XC_PAGE_SHIFT,
- __test_bit_size XC_PAGE_SHIFT,
+ test_bit_size XC_PAGE_SHIFT,
mapcache-last_entry-valid_mapping)) {
trace_xen_map_cache_return(mapcache-last_entry-vaddr_base +
address_offset);
return mapcache-last_entry-vaddr_base + address_offset;
@@ -232,20 +232,20 @@ tryagain:
/* size is always a multiple of MCACHE_BUCKET_SIZE */
if (size) {
-__size = size + address_offset;
-if (__size % MCACHE_BUCKET_SIZE) {
-__size += MCACHE_BUCKET_SIZE - (__size % MCACHE_BUCKET_SIZE);
+cache_size = size + address_offset;
+if (cache_size % MCACHE_BUCKET_SIZE) {
+cache_size += MCACHE_BUCKET_SIZE - (cache_size %
MCACHE_BUCKET_SIZE);
}
} else {
-__size = MCACHE_BUCKET_SIZE;
+cache_size = MCACHE_BUCKET_SIZE;
}
entry = mapcache-entry[address_index % mapcache-nr_buckets];
while (entry entry-lock entry-vaddr_base
-(entry-paddr_index != address_index || entry-size != __size ||
+(entry-paddr_index != address_index || entry-size != cache_size
||
!test_bits(address_offset XC_PAGE_SHIFT,
- __test_bit_size XC_PAGE_SHIFT,
+ test_bit_size XC_PAGE_SHIFT,
entry-valid_mapping))) {
pentry = entry;
entry = entry-next;
@@ -253,19 +253,19 @@ tryagain:
if (!entry) {
entry = g_malloc0(sizeof (MapCacheEntry));
pentry-next = entry;
-xen_remap_bucket(entry, __size, address_index);
+xen_remap_bucket(entry, cache_size, address_index);
} else if (!entry-lock) {
if (!entry-vaddr_base || entry-paddr_index != address_index ||
-entry-size != __size ||
+entry-size != cache_size ||
!test_bits(address_offset XC_PAGE_SHIFT,
-__test_bit_size XC_PAGE_SHIFT,
+test_bit_size XC_PAGE_SHIFT,
entry-valid_mapping)) {
-xen_remap_bucket(entry, __size, address_index);
+xen_remap_bucket(entry, cache_size, address_index);
}
}
if(!test_bits(address_offset XC_PAGE_SHIFT,
-__test_bit_size XC_PAGE_SHIFT,
+test_bit_size XC_PAGE_SHIFT,
entry-valid_mapping)) {
mapcache-last_entry = NULL;
if (!translated mapcache-phys_offset_to_gaddr) {
--
1.7.10.4
Re: [Qemu-devel] [PATCH 0/2] target-s390x: fixes for GMP testsuite
On 08/01/2015 18:01, Paolo Bonzini wrote: Reported and tested by Torbjorn. One patch is mine, one is his. Paolo Bonzini (1): target-s390x: support OC and NC in the EX instruction Torbjorn Granlund (1): target-s390x: fix and optimize slb* and slbg* computation of carry/borrow flag target-s390x/cc_helper.c | 18 -- target-s390x/mem_helper.c | 8 2 files changed, 12 insertions(+), 14 deletions(-) Alex, ping? Paolo
[Qemu-devel] Exposing different e1000 models through cmdline
Hi,
I found that vmware emulates a 82545em per default and so some special crafted
appliances only
work with that card. I was wondering if the below is the right approach to make
the
models selectable via cmdline without changing the default:
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 371699c..632c4ba 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1578,6 +1578,9 @@ static const char * const pci_nic_models[] = {
i82559er,
rtl8139,
e1000,
+e1000-82540em,
+e1000-82544gc,
+e1000-82545em,
pcnet,
virtio,
NULL
@@ -1590,6 +1593,9 @@ static const char * const pci_nic_names[] = {
i82559er,
rtl8139,
e1000,
+e1000-82540em,
+e1000-82544gc,
+e1000-82545em,
pcnet,
virtio-net-pci,
NULL
Thanks,
Peter
Re: [Qemu-devel] [PATCH v8] block/raw-posix.c: Fix raw_getlength() on Mac OS X for CD
On Jan 20, 2015, at 3:33 AM, Markus Armbruster wrote:
Programmingkid programmingk...@gmail.com writes:
Subject was:
Re: [PATCH v7] block/raw-posix.c: Fixes raw_getlength()
on Mac OS X so that it reports the correct length of a real CD
Patch history information goes...
This patch allows Mac OS X to use a real CDROM disc in QEMU.
Testing this patch will require using QEMU v2.2.0 because the
current git version has a bug in it that prevents /dev/cdrom from
being used. make check did pass and my Debian boot disc did work.
Signed-off-by: John Arbuckle programmingk...@gmail.com
---
... below the --- divider.
I thought I did this. The information above is the description of the patch.
Not its history.
Fixed code indentation to be inline with removed
size = LLONG_MAX.
block/raw-posix.c | 15 ++-
1 files changed, 14 insertions(+), 1 deletions(-)
diff --git a/block/raw-posix.c b/block/raw-posix.c
index e51293a..fa431b2 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1312,7 +1312,20 @@ again:
if (size == 0)
#endif
#if defined(__APPLE__) defined(__MACH__)
-size = LLONG_MAX;
+{
+uint64_t sectors = 0;
+uint32_t sector_size = 0;
Ignorant question: why do you need to initialize these?
We don't. It's just a habit.
Patch looks plausible otherwise, but know nothing about Macs :)
It does do the job.
+
+if (ioctl(fd, DKIOCGETBLOCKCOUNT, sectors) == 0
+ioctl(fd, DKIOCGETBLOCKSIZE, sector_size) == 0) {
+size = sectors * sector_size;
+} else {
+size = lseek(fd, 0LL, SEEK_END);
+if (size 0) {
+return -errno;
+}
+}
+}
#else
size = lseek(fd, 0LL, SEEK_END);
if (size 0) {
Re: [Qemu-devel] KVM call for agenda for 2015-01-20
Juan Quintela quint...@redhat.com wrote: Hi Please, send any topic that you are interested in covering. Thanks, Juan. Call details: By popular demand, a google calendar public entry with it As there are no agenda, call gets cancelled. Sorry for the late notification. https://www.google.com/calendar/embed?src=dG9iMXRqcXAzN3Y4ZXZwNzRoMHE4a3BqcXNAZ3JvdXAuY2FsZW5kYXIuZ29vZ2xlLmNvbQ (Let me know if you have any problems with the calendar entry. I just gave up about getting right at the same time CEST, CET, EDT and DST). If you need phone number details, contact me privately Thanks, Juan.
Re: [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions
Hi, This looks like a list for GL_TRIANGLES instead of GL_TRIANGLE_STRIP. For GL_TRIANGLE_STRIP, you first define one whole triangle (by specifying each of the three vertices) and after that, each vertex results in a new triangle drawn (whose two other vertices are the two vertices preceding the last one). Thanks for the nice description. So the trick to get it done with only four vertexes is to put the correct points (which are going to be shared by both triangles) into the middle. I've played around with it a bit without success (and before my new opengl book arrived), and this 6-point version worked ... Hm, I did write a working solution in my reply, didn't I? Yep, was just trying to explain how I ended up with the 6 vertex version, not having fully figured how triangle strips are working. All fine now, I've changed it to use 4 vertexes version instead, and I _also_ understood why it is working as intended. FYI: current code is at https://www.kraxel.org/cgit/qemu/log/?h=rebase/ui-sdl-next [ incremental fixes not squashed in yet ] thanks, Gerd
Re: [Qemu-devel] [PATCH 4/7] ppc: move sdr1 value change detection logic to helper_store_sdr1()
On 23/12/2014 01:36, Mark Cave-Ayland wrote:
Otherwise when cpu_post_load calls ppc_store_sdr1() when restoring a VM
snapshot the value is deemed unchanged and so the internal env-htab*
variables aren't set correctly.
Signed-off-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk
CC: Paolo Bonzini pbonz...@redhat.com
---
target-ppc/misc_helper.c |7 ++-
target-ppc/mmu_helper.c | 35 +++
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/target-ppc/misc_helper.c b/target-ppc/misc_helper.c
index a577b3a..6b12ca8 100644
--- a/target-ppc/misc_helper.c
+++ b/target-ppc/misc_helper.c
@@ -77,8 +77,13 @@ void helper_msr_facility_check(CPUPPCState *env, uint32_t
bit,
void helper_store_sdr1(CPUPPCState *env, target_ulong val)
{
+PowerPCCPU *cpu = ppc_env_get_cpu(env);
+
if (!env-external_htab) {
-ppc_store_sdr1(env, val);
+if (env-spr[SPR_SDR1] != val) {
+ppc_store_sdr1(env, val);
+tlb_flush(CPU(cpu), 1);
Possibly stupid question: should this tlb_flush be in ppc_store_sdr1,
maybe guarded by if (tcg_enabled())?
Apart from this, the patch is okay.
Paolo
+}
}
}
diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index 660be7f..527c6ad 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -2036,31 +2036,26 @@ void ppc_tlb_invalidate_one(CPUPPCState *env,
target_ulong addr)
/* Special registers manipulation */
void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
{
-PowerPCCPU *cpu = ppc_env_get_cpu(env);
-
qemu_log_mask(CPU_LOG_MMU, %s: TARGET_FMT_lx \n, __func__, value);
assert(!env-external_htab);
-if (env-spr[SPR_SDR1] != value) {
-env-spr[SPR_SDR1] = value;
+env-spr[SPR_SDR1] = value;
#if defined(TARGET_PPC64)
-if (env-mmu_model POWERPC_MMU_64) {
-target_ulong htabsize = value SDR_64_HTABSIZE;
+if (env-mmu_model POWERPC_MMU_64) {
+target_ulong htabsize = value SDR_64_HTABSIZE;
-if (htabsize 28) {
-fprintf(stderr, Invalid HTABSIZE 0x TARGET_FMT_lx
- stored in SDR1\n, htabsize);
-htabsize = 28;
-}
-env-htab_mask = (1ULL (htabsize + 18 - 7)) - 1;
-env-htab_base = value SDR_64_HTABORG;
-} else
-#endif /* defined(TARGET_PPC64) */
-{
-/* FIXME: Should check for valid HTABMASK values */
-env-htab_mask = ((value SDR_32_HTABMASK) 16) | 0x;
-env-htab_base = value SDR_32_HTABORG;
+if (htabsize 28) {
+fprintf(stderr, Invalid HTABSIZE 0x TARGET_FMT_lx
+ stored in SDR1\n, htabsize);
+htabsize = 28;
}
-tlb_flush(CPU(cpu), 1);
+env-htab_mask = (1ULL (htabsize + 18 - 7)) - 1;
+env-htab_base = value SDR_64_HTABORG;
+} else
+#endif /* defined(TARGET_PPC64) */
+{
+/* FIXME: Should check for valid HTABMASK values */
+env-htab_mask = ((value SDR_32_HTABMASK) 16) | 0x;
+env-htab_base = value SDR_32_HTABORG;
}
}
Re: [Qemu-devel] [PATCH 5/7] ppc: force update of all msr bits in cpu_post_load
On 23/12/2014 01:36, Mark Cave-Ayland wrote:
Since env-msr has already been restored by the time cpu_post_load is called,
make sure that ppc_store_msr() is explicitly called with all msr bits marked
as invalid.
This solves the issue where MSR flags aren't set correctly when restoring a VM
snapshot, in particular the internal env-excp_prefix value when MSR_EP has
been altered by a guest.
Signed-off-by: Mark Cave-Ayland mark.cave-ayl...@ilande.co.uk
---
target-ppc/machine.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index c801b82..9669063 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -159,6 +159,7 @@ static int cpu_post_load(void *opaque, int version_id)
PowerPCCPU *cpu = opaque;
CPUPPCState *env = cpu-env;
int i;
+target_ulong msr;
/*
* We always ignore the source PVR. The user or management
@@ -190,7 +191,12 @@ static int cpu_post_load(void *opaque, int version_id)
/* Restore htab_base and htab_mask variables */
ppc_store_sdr1(env, env-spr[SPR_SDR1]);
}
-hreg_compute_hflags(env);
+
+/* Mark all msr bits invalid before restoring */
+msr = env-msr;
+env-msr = ~env-msr;
+ppc_store_msr(env, msr);
+
hreg_compute_mem_idx(env);
return 0;
This is wrong for this line of hreg_store_msr:
if (unlikely((env-flags POWERPC_FLAG_TGPR)
((value ^ env-msr) (1 MSR_TGPR {
/* Swap temporary saved registers with GPRs */
hreg_swap_gpr_tgpr(env);
}
I think you need to change the ~ with ^ ~(1 MSR_TGPR).
Paolo
Re: [Qemu-devel] [PATCH 0/2] qcow2: Add two more unalignment checks
On 2015-01-20 at 05:09, Kevin Wolf wrote: Am 19.01.2015 um 22:09 hat Max Reitz geschrieben: On 2015-01-19 at 16:04, Eric Blake wrote: On 01/19/2015 01:49 PM, Max Reitz wrote: With the series adding unalignment checks and the series reworking the zero cluster expansion code overlapping, the unalignment checks have not been implemented in the latter code. This series fixes it. There are other places which would require unalignment checks, like the offsets of L1 tables, especially for snapshots; but because it would be best to add these checks in the function which reads the snapshot table, this would make images with broken snapshots completely unusable, which is something I opted to avoid for now. That's something I noticed, too: For qemu-img check, our qcow2_open() checks may be to strict. With a corrupted snapshot table, it won't even run. Perhaps we should be laxer with some checks with BDRV_O_CHECK, and for example just set s-nb_snapshots = 0 if loading the table fails. Ideally, we need to make the qcow2 repair function repair such cases, but until that is done there is not much we can do about them. What's the best repair? That's what I was asking myself... Read the data from the unaligned location, and write a fresh copy into a new aligned allocation? Maybe. Maybe there is no way of repairing them and the only way is asking the user whether it's fine to delete the snapshot (qemu-img check -r make_consistent_whatever_it_takes). Also, the question remains for every unaligned data structure: L2 tables, data clusters… The refcount structures are the only ones that can be easily recovered. For data clusters, reading from the unaligned location would probably be best; for L2 tables… maybe the same, then run a consistency check on the data and if it seems off™, throw the whole L2 table away. Reading from the unaligned location doesn't help. I have never seen an image whose L2 entries contained valid entries, except for the least significant few bits. If your table is corrupted, it's usually garbage from start to end. Probably so. I think the only sane option is to drop the entries. The big question is what should be used to replace them. /dev/random of course. There is a chance we are correct… Seriously speaking, well, unmap them? Zero clusters don't feel right to me. Max
Re: [Qemu-devel] Exposing different e1000 models through cmdline
On Tue, Jan 20, 2015 at 03:08:04PM +0100, Peter Lieven wrote:
Hi,
I found that vmware emulates a 82545em per default and so some special
crafted appliances only
work with that card. I was wondering if the below is the right approach to
make the
models selectable via cmdline without changing the default:
No - please just use -device to create devices.
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 371699c..632c4ba 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1578,6 +1578,9 @@ static const char * const pci_nic_models[] = {
i82559er,
rtl8139,
e1000,
+e1000-82540em,
+e1000-82544gc,
+e1000-82545em,
pcnet,
virtio,
NULL
@@ -1590,6 +1593,9 @@ static const char * const pci_nic_names[] = {
i82559er,
rtl8139,
e1000,
+e1000-82540em,
+e1000-82544gc,
+e1000-82545em,
pcnet,
virtio-net-pci,
NULL
Thanks,
Peter
Re: [Qemu-devel] [PATCH v2 4/7] console-gl: add opengl rendering helper functions
On 2015-01-20 at 06:00, Gerd Hoffmann wrote:
Hi,
+static GLchar texture_blit_vert_src[] =
+\n
+#version 300 es\n
+\n
+in vec2 in_position;\n
+in vec2 in_tex_coord;\n
You could calculate the texture coordinate from the position in the
shader, but this is mostly my premature optimization instinct kicking in
instead of a real performance difference considering how few vertices
there are in this case.
Still makes sense. Done.
+out vec2 ex_tex_coord;\n
+\n
+void main(void) {\n
+gl_Position = vec4(in_position.x, in_position.y, 0.0, 1.0);\n
vec4(in_position, 0.0, 1.0) *cough*
Done.
This looks like a list for GL_TRIANGLES instead of GL_TRIANGLE_STRIP.
For GL_TRIANGLE_STRIP, you first define one whole triangle (by
specifying each of the three vertices) and after that, each vertex
results in a new triangle drawn (whose two other vertices are the two
vertices preceding the last one).
Thanks for the nice description.
So the trick to get it done with only four vertexes is to put the
correct points (which are going to be shared by both triangles) into the
middle. I've played around with it a bit without success (and before my
new opengl book arrived), and this 6-point version worked ...
Hm, I did write a working solution in my reply, didn't I? It was: { -1,
-1, 1, -1, -1, 1, 1, 1 } for in_position[] and { 0, 1, 1, 1, 0,
0, 1, 0 } for in_tex_coord[]. At least it worked for me.
You haven't (explicitly) enabled face culling, but remember that while
normally counter-clockwise faces are backfaces and thus culled, the
direction is inverted for every triangle; so the first triangle needs to
be CCW, the second CW, the third CCW again, and so on. Maybe that's why
it didn't work for you...
+glUseProgram(gls-texture_blit_prog);
+l_position = glGetAttribLocation(gls-texture_blit_prog, in_position);
+l_tex_coord = glGetAttribLocation(gls-texture_blit_prog, in_tex_coord);
+glVertexAttribPointer(l_position, 2, GL_FLOAT, GL_FALSE, 0, in_position);
+glVertexAttribPointer(l_tex_coord, 2, GL_FLOAT, GL_FALSE, 0, in_tex_coord);
I think it is disallowed to refer to non-OpenGL buffers here in the core
profile. The 4.4 core specification states (section 10.3, vertex
arrays): Vertex data are placed into arrays that are stored in the
server’s address space; the 4.4 compatibility specification states:
Vertex data may also be placed into arrays that are stored in the
client’s address space (described here) or in the server’s address space.
Got this from gles sample code, so that should be ok ;)
I've also seen code explicitly storing vertex data in a buffer, which I
assume is more efficient, but I decided to not care for now, especially
given the small number of vertexes we are processing here.
+return gl_create_link_program(vert_shader, frag_shader);
Minor thing: You are free to delete the shaders after they have been
linked into the program (because the references will be lost when this
function returns).
Done.
+switch (surface-format) {
+case PIXMAN_BE_b8g8r8x8:
+case PIXMAN_BE_b8g8r8a8:
+surface-glformat = GL_BGRA_EXT;
If you want to avoid the _EXT, you could use GL_RGBA here and
texture().bgra in the fragment shader. However, this would require a
different shader for the 32 bit and the 16 bit formats (because the 16
bit format has the right order, apparently).
At least it worked in testing.
I haven't been able to really test 16 bit mode (forcing 16 bit mode in
hw/display/vga.c doesn't count...), so I'm trusting you this works.
-kernel /boot/vmlinux-$whatever -append vga=0x314
Makes the kernel run vesafb with 800x600 @ 16bpp, and thanks to the
little friendly penguin in the upper left corner (CONFIG_LOGO=y) you can
easily spot colors being messed up.
Thanks!
Max
Re: [Qemu-devel] [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend driver
-Original Message-
From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
Sent: Tuesday, January 20, 2015 6:50 PM
To: Xu, Quan
Cc: Stefano Stabellini; qemu-devel@nongnu.org; xen-de...@lists.xen.org
Subject: RE: [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM frontend
driver
On Tue, 20 Jan 2015, Xu, Quan wrote:
-Original Message-
From: Stefano Stabellini [mailto:stefano.stabell...@eu.citrix.com]
Sent: Tuesday, January 20, 2015 1:15 AM
To: Xu, Quan
Cc: qemu-devel@nongnu.org; xen-de...@lists.xen.org;
stefano.stabell...@eu.citrix.com
Subject: Re: [v3 3/5] Qemu-Xen-vTPM: Register Xen stubdom vTPM
frontend driver
On Tue, 30 Dec 2014, Quan Xu wrote:
This drvier transfers any request/repond between TPM xenstubdoms
driver and Xen vTPM stubdom, and facilitates communications
between Xen vTPM stubdom domain and vTPM xenstubdoms driver. It is
a glue for the TPM xenstubdoms driver and Xen stubdom vTPM domain
that provides the actual TPM functionality.
(Xen) Xen backend driver should run before running this frontend,
and initialize XenStore as the following for communication.
[XenStore]
..
FE.DOMAIN.ID
device =
vtpm =
0 =
backend =
/local/domain/{BE.DOMAIN.ID}/backend/vtpm/{FE.DOMAIN.ID}/0
backend-id = BE.DOMAIN.ID
state = 1
handle = 0
..
(QEMU) xen_vtpmdev_ops is initialized with the following process:
xen_hvm_init()
[...]
--xen_fe_register(vtpm, ...)
--xenstore_fe_scan()
--xen_fe_get_xendev()
-- XenDevOps.alloc()
--xen_fe_check()
-- XenDevOps.init()
-- XenDevOps.initialise()
-- XenDevOps.connected()
--xs_watch()
[...]
--Changes in v3:
-Move xen_stubdom_vtpm.c to xen_vtpm_frontend.c -Read Xen vTPM
status via XenStore
Signed-off-by: Quan Xu quan...@intel.com
---
hw/tpm/Makefile.objs | 1 +
hw/tpm/xen_vtpm_frontend.c | 264
+++
hw/xen/xen_backend.c | 34 ++
include/hw/xen/xen_backend.h | 9 +-
include/hw/xen/xen_common.h | 6 +
xen-hvm.c| 16 +++
6 files changed, 328 insertions(+), 2 deletions(-) create mode
100644 hw/tpm/xen_vtpm_frontend.c
diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs index
99f5983..57919fa 100644
--- a/hw/tpm/Makefile.objs
+++ b/hw/tpm/Makefile.objs
@@ -1,2 +1,3 @@
common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
+common-obj-$(CONFIG_TPM_XENSTUBDOMS) += xen_vtpm_frontend.o
diff --git a/hw/tpm/xen_vtpm_frontend.c
b/hw/tpm/xen_vtpm_frontend.c new file mode 100644 index
000..00cc888
--- /dev/null
+++ b/hw/tpm/xen_vtpm_frontend.c
@@ -0,0 +1,264 @@
+/*
+ * Connect to Xen vTPM stubdom domain
+ *
+ * Copyright (c) 2014 Intel Corporation
+ * Authors:
+ *Quan Xu quan...@intel.com
+ *
+ * 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 library is distributed in the hope that it will be
+useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General
+Public
+ * License along with this library; if not, see
+http://www.gnu.org/licenses/ */
+
+#include stdio.h
+#include stdlib.h
+#include stdarg.h
+#include string.h
+#include unistd.h
+#include signal.h
+#include inttypes.h
+#include time.h
+#include fcntl.h
+#include errno.h
+#include sys/ioctl.h
+#include sys/types.h
+#include sys/stat.h
+#include sys/mman.h
+#include sys/uio.h
+
+#include hw/hw.h
+#include block/aio.h
+#include hw/xen/xen_backend.h
+
+enum tpmif_state {
+TPMIF_STATE_IDLE,/* no contents / vTPM idle / cancel
complete */
+TPMIF_STATE_SUBMIT, /* request ready / vTPM working */
+TPMIF_STATE_FINISH, /* response ready / vTPM idle */
+TPMIF_STATE_CANCEL, /* cancel requested / vTPM working */
+};
+
+static AioContext *vtpm_aio_ctx;
+
+enum status_bits {
+VTPM_STATUS_RUNNING = 0x1,
+VTPM_STATUS_IDLE = 0x2,
+VTPM_STATUS_RESULT = 0x4,
+VTPM_STATUS_CANCELED = 0x8,
+};
+
+struct tpmif_shared_page {
+uint32_t
Re: [Qemu-devel] [PATCH 2/3 V3] s390: implement pci instructions
On Tue, Jan 20, 2015 at 01:56:09PM +0100, Markus Armbruster wrote:
Markus Armbruster arm...@redhat.com writes:
Cornelia Huck cornelia.h...@de.ibm.com writes:
On Tue, 20 Jan 2015 10:45:41 +0100
Markus Armbruster arm...@redhat.com wrote:
This patch makes Coverity unhappy:
*** CID 1264326: Unintended sign extension (SIGN_EXTENSION)
/hw/s390x/s390-pci-inst.c: 787 in stpcifc_service_call()
781 stq_p(fib.pal, pbdev-pal);
782 stq_p(fib.iota, pbdev-g_iota);
783 stq_p(fib.aibv, pbdev-routes.adapter.ind_addr);
784 stq_p(fib.aisb, pbdev-routes.adapter.summary_addr);
785 stq_p(fib.fmb_addr, pbdev-fmb_addr);
786
CID 1264326: Unintended sign extension (SIGN_EXTENSION)
Suspicious implicit sign extension: pbdev-isc with type
unsigned char (8 bits, unsigned) is promoted in (pbdev-isc
28) | (pbdev-noi 16) to type int (32 bits, signed), then
sign-extended to type unsigned long (64 bits, unsigned). If
(pbdev-isc 28) | (pbdev-noi 16) is greater than
0x7FFF, the upper bits of the result will all be 1.
787 data = (pbdev-isc 28) | (pbdev-noi 16) |
788 (pbdev-routes.adapter.ind_offset 8) | (pbdev-sum 7) |
789pbdev-routes.adapter.summary_offset;
790 stw_p(fib.data, data);
791
792 if (pbdev-fh ENABLE_BIT_OFFSET) {
There's a fix for this (and the memory leak):
http://marc.info/?l=qemu-develm=142124886620078w=2
The patch is sitting in my queue, will send with the next pile of s390x
updates.
I can't see how
@@ -787,7 +787,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1,
uint64_t fiba)
data = (pbdev-isc 28) | (pbdev-noi 16) |
(pbdev-routes.adapter.ind_offset 8) | (pbdev-sum 7) |
pbdev-routes.adapter.summary_offset;
-stw_p(fib.data, data);
+stl_p(fib.data, data);
if (pbdev-fh ENABLE_BIT_OFFSET) {
fib.fc |= 0x80;
fixes the implicit sign extension within the assignment preceding it.
Let me explain it again real slow:
1. pbdev-isc gets promoted from uint8_t to int as operand of binary
(usual arithmetic conversions ISO/IEC 9899:1999 6.3.1.8)
2. The int result is shifted left 28 bits. This can set the MSB.
3. Likewise: pbdev-noi gets promoted from uint64_t to int, and shifted
left 16 bits.
uint16_t to int
4. The two shift results stay int and get ored.
5. pbdev-routes.adapter.ind_offset stays uint64_t, and is shifted left
8 bits.
6. The next or's left operand is the int result of 4 and the right
operant is the uint64_t result of 5. Therefore, the left operand is
*sign-extended* from int to uint64_t. This copies bit#7 of
pbdev-isc to bits#31..63. Whoops.
I neglected to say: we don't currently use the upper 32 bits, and as
long as we do that, the sign extension is harmless. I'd recommend to
avoid it all the same, for robustness, and to hush up Coverity.
Hi Markus,
thx for your explanation. I did not see a problem since ISC is not bigger
than 0x7 so MSB is never set. But the time I wrote the code I was not aware of
ind_offset is uint64_t since zpci defines only a 6 bit field for this value.
How can I avoid the sign extension and make Coverity happy?
Regarding the leak, I prefer my patch, because it avoids the free on
error. But you're the maintainer.
This is fine for me as well ...
Thx,
Frank
Re: [Qemu-devel] [PULL 0/4] Xen tree 2015-01-20
On Tue, 20 Jan 2015, Peter Maydell wrote: On 20 January 2015 at 11:19, Stefano Stabellini stefano.stabell...@eu.citrix.com wrote: The following changes since commit 74acb99737dbedd86654d660c0c20815139a873c: Merge remote-tracking branch 'remotes/kraxel/tags/pull-console-20150119-1' into staging (2015-01-19 13:37:05 +) are available in the git repository at: git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-2015-01-20 for you to fetch changes up to 085bde8e8f9bd4fb06e010810991b26aba795fb2: xen: add a lock for the mapcache (2015-01-20 11:09:54 +) I'm afraid I can't apply this -- three out of four patches are missing your signed-off-by as maintainer. You might like to add something to your pre-submission checking scripts to catch this error; eg the script fragment I suggested in http://lists.nongnu.org/archive/html/qemu-devel/2015-01/msg02134.html No problem, I'll resend shortly.
[Qemu-devel] [PATCH 2/3] Migration: Add lots of trace events
From: Dr. David Alan Gilbert dgilb...@redhat.com
Mostly on the load side, so that when we get a complaint about
a migration failure we can figure out what it didn't like.
Signed-off-by: Dr. David Alan Gilbert dgilb...@redhat.com
---
migration/vmstate.c | 22 +++---
savevm.c| 10 +++---
trace-events| 11 ++-
3 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 2c0b135..7b8dc3f 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -75,14 +75,19 @@ int vmstate_load_state(QEMUFile *f, const
VMStateDescription *vmsd,
VMStateField *field = vmsd-fields;
int ret;
+trace_vmstate_load_state(vmsd-name, version_id);
if (version_id vmsd-version_id) {
+trace_vmstate_load_state_end(vmsd-name, too new, -EINVAL);
return -EINVAL;
}
if (version_id vmsd-minimum_version_id) {
if (vmsd-load_state_old
version_id = vmsd-minimum_version_id_old) {
-return vmsd-load_state_old(f, opaque, version_id);
+ret = vmsd-load_state_old(f, opaque, version_id);
+trace_vmstate_load_state_end(vmsd-name, old path, ret);
+return ret;
}
+trace_vmstate_load_state_end(vmsd-name, too old, -EINVAL);
return -EINVAL;
}
if (vmsd-pre_load) {
@@ -92,6 +97,7 @@ int vmstate_load_state(QEMUFile *f, const VMStateDescription
*vmsd,
}
}
while (field-name) {
+trace_vmstate_load_state_field(vmsd-name, field-name);
if ((field-field_exists
field-field_exists(opaque, version_id)) ||
(!field-field_exists
@@ -134,9 +140,10 @@ int vmstate_load_state(QEMUFile *f, const
VMStateDescription *vmsd,
return ret;
}
if (vmsd-post_load) {
-return vmsd-post_load(opaque, version_id);
+ret = vmsd-post_load(opaque, version_id);
}
-return 0;
+trace_vmstate_load_state_end(vmsd-name, end, ret);
+return ret;
}
void vmstate_save_state(QEMUFile *f, const VMStateDescription *vmsd,
@@ -193,6 +200,8 @@ static const VMStateDescription *
static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque)
{
+trace_vmstate_subsection_load(vmsd-name);
+
while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
char idstr[256];
int ret;
@@ -202,20 +211,24 @@ static int vmstate_subsection_load(QEMUFile *f, const
VMStateDescription *vmsd,
len = qemu_peek_byte(f, 1);
if (len strlen(vmsd-name) + 1) {
/* subsection name has be be section_name/a */
+trace_vmstate_subsection_load_bad(vmsd-name, (short));
return 0;
}
size = qemu_peek_buffer(f, (uint8_t *)idstr, len, 2);
if (size != len) {
+trace_vmstate_subsection_load_bad(vmsd-name, (peek fail));
return 0;
}
idstr[size] = 0;
if (strncmp(vmsd-name, idstr, strlen(vmsd-name)) != 0) {
+trace_vmstate_subsection_load_bad(vmsd-name, idstr);
/* it don't have a valid subsection name */
return 0;
}
sub_vmsd = vmstate_get_subsection(vmsd-subsections, idstr);
if (sub_vmsd == NULL) {
+trace_vmstate_subsection_load_bad(vmsd-name, (lookup));
return -ENOENT;
}
qemu_file_skip(f, 1); /* subsection */
@@ -225,9 +238,12 @@ static int vmstate_subsection_load(QEMUFile *f, const
VMStateDescription *vmsd,
ret = vmstate_load_state(f, sub_vmsd, opaque, version_id);
if (ret) {
+trace_vmstate_subsection_load_bad(vmsd-name, (child));
return ret;
}
}
+
+trace_vmstate_subsection_load_good(vmsd-name);
return 0;
}
diff --git a/savevm.c b/savevm.c
index 48c2396..1cc0f02 100644
--- a/savevm.c
+++ b/savevm.c
@@ -674,7 +674,7 @@ int qemu_savevm_state_iterate(QEMUFile *f)
qemu_put_be32(f, se-section_id);
ret = se-ops-save_live_iterate(f, se-opaque);
-trace_savevm_section_end(se-idstr, se-section_id);
+trace_savevm_section_end(se-idstr, se-section_id, ret);
if (ret 0) {
qemu_file_set_error(f, ret);
@@ -714,7 +714,7 @@ void qemu_savevm_state_complete(QEMUFile *f)
qemu_put_be32(f, se-section_id);
ret = se-ops-save_live_complete(f, se-opaque);
-trace_savevm_section_end(se-idstr, se-section_id);
+trace_savevm_section_end(se-idstr, se-section_id, ret);
if (ret 0) {
qemu_file_set_error(f, ret);
return;
@@ -741,7 +741,7 @@ void qemu_savevm_state_complete(QEMUFile *f)
qemu_put_be32(f, se-version_id);
vmstate_save(f, se);
-trace_savevm_section_end(se-idstr, se-section_id);
+trace_savevm_section_end(se-idstr, se-section_id, 0);
}
Re: [Qemu-devel] [PATCH RFC v6 16/20] virtio-net: support longer header
On Thu, Dec 11, 2014 at 02:25:18PM +0100, Cornelia Huck wrote: diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index ebbea60..7ee2bd6 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -373,15 +373,21 @@ static int peer_has_ufo(VirtIONet *n) return n-has_ufo; } -static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs) +static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs, + int version_1) Please use bool, it makes it 100% clear what the meaning of version_1 is. s/int/bool/ pgp36dTzGpTkZ.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing
Greg Bellows greg.bell...@linaro.org writes:
Adds a CPU feature parsing function and assigns to the CPU class. The only
feature added was -aarch64 which disabled the AArch64 execution state on a
64-bit ARM CPU.
Also adds stripping of features from CPU model string in acquiring the ARM CPU
by name.
Signed-off-by: Greg Bellows greg.bell...@linaro.org
---
target-arm/cpu.c | 45 -
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 285947f..f327dd7 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -514,13 +514,17 @@ static ObjectClass *arm_cpu_class_by_name(const char
*cpu_model)
{
ObjectClass *oc;
char *typename;
+char *cpuname;
if (!cpu_model) {
return NULL;
}
-typename = g_strdup_printf(%s- TYPE_ARM_CPU, cpu_model);
+cpuname = g_strdup(cpu_model);
+cpuname = strtok(cpuname, ,);
+typename = g_strdup_printf(%s- TYPE_ARM_CPU, cpuname);
oc = object_class_by_name(typename);
+g_free(cpuname);
g_free(typename);
Aren't we leaking here? strtok returns the next token (or NULL) so don't
we loose the original ptr?
Also while using glib you might want to consider using glib's own
tokenising functions (e.g. g_strsplit). This has the advantage of having
helper functions like g_strfreev() which can clean-up everything in one go.
if (!oc || !object_class_dynamic_cast(oc, TYPE_ARM_CPU) ||
object_class_is_abstract(oc)) {
@@ -1163,6 +1167,44 @@ static Property arm_cpu_properties[] = {
DEFINE_PROP_END_OF_LIST()
};
+static void arm_cpu_parse_features(CPUState *cs, char *features,
+ Error **errp)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+char *featurestr;
+
+featurestr = features ? strtok(features, ,) : NULL;
+while (featurestr) {
+if (featurestr[0] == '-') {
+if (!strcmp(featurestr+1, aarch64)) {
+/* If AArch64 is disabled then we need to unset the feature
*/
+unset_feature(cpu-env, ARM_FEATURE_AARCH64);
+} else {
+/* Everyting else is unsupported */
+error_setg(errp, unsupported CPU property '%s',
+ featurestr[1]);
+return;
+}
+} else if (featurestr[0] == '+') {
+/* No '+' properties supported yet */
+error_setg(errp, unsupported CPU property '%s',
+ featurestr[1]);
+return;
+} else if (g_strstr_len(featurestr, -1, =)) {
+/* No '=' properties supported yet */
+char *prop = strtok(featurestr, =);
+error_setg(errp, unsupported CPU property '%s', prop);
+return;
+} else {
+/* Everything else is a bad format */
+error_setg(errp, CPU property string '%s' not in format
+ (+feature|-feature|feature=xyz), featurestr);
+return;
+}
+featurestr = strtok(NULL, ,);
+}
+}
I only point to this for reference to a gliby approach to the parsing,
relative beauty being in the eye of the beholder ;-)
https://github.com/stsquad/qemu/commit/86bc88f661141b93cbe5b107c4d5b4322b563241#diff-286aa0f2c1f0d862c4197781280a92efR116
It does make me think boilerplate but I wonder if this is a generic
enough problem to be solved more generally in QEMU?
+
static void arm_cpu_class_init(ObjectClass *oc, void *data)
{
ARMCPUClass *acc = ARM_CPU_CLASS(oc);
@@ -1183,6 +1225,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void
*data)
cc-set_pc = arm_cpu_set_pc;
cc-gdb_read_register = arm_cpu_gdb_read_register;
cc-gdb_write_register = arm_cpu_gdb_write_register;
+cc-parse_features = arm_cpu_parse_features;
#ifdef CONFIG_USER_ONLY
cc-handle_mmu_fault = arm_cpu_handle_mmu_fault;
#else
--
Alex Bennée
Re: [Qemu-devel] [PATCH 0/2] m48t59: add year offset and MMIO ISA mapping
On 19/01/15 21:59, Hervé Poussineau wrote: Le 19/01/2015 12:35, Mark Cave-Ayland a écrit : This patch lays the groundwork for switching sun4u over from ioport NVRAM access to MMIO NVRAM access. Patch 1 introduces a new year_offset property which is the offset between the year value stored in hardware and the actual year. In particular, Sun hardware has a 68 year offset used to extend the date range of the IC. While the kernel sources I have viewed contain this offset within a #ifdef CONFIG_SPARC block, I've updated all the callers so that no change in behaviour will be seen across all platforms. PPC users may wish to alter the callers to change this behaviour as required. Patch 2 mimics the mem_base parameter from m48t59_init() to m48t59_init_isa() so that MMIO can be used for sun4u where the NVRAM is attached to the ebus which is essentially the same as an ISA bus. I've a patch which QOM'ifies m48t59, that I'll send to the list. If you apply it, you'll be then able to create a sysbus-m48t02 device, and then to add it to ebus memory region. IMO, there is no need to add a new parameter to m48t59_init_isa device. Tell me what do you think of it. It took me a while to go through these patches, but yes I think that would work (i.e. create the sysbus-m48t59 device and then map it's MemoryRegion directly into I/O space). If these patches can be applied then I'm happy to rebase and resubmit my patchset for the year_offset property. Who's tree should these changes go through? Andreas' QOM tree? ATB, Mark.
[Qemu-devel] [PULL v2 0/4] Xen tree 2015-01-20 v2
The following changes since commit 74acb99737dbedd86654d660c0c20815139a873c: Merge remote-tracking branch 'remotes/kraxel/tags/pull-console-20150119-1' into staging (2015-01-19 13:37:05 +) are available in the git repository at: git://xenbits.xen.org/people/sstabellini/qemu-dm.git xen-2015-01-20-v2 for you to fetch changes up to 86a6a9bf551ffa183880480b37c5836d3916687a: xen: add a lock for the mapcache (2015-01-20 14:24:17 +) Paolo Bonzini (2): xen: do not use __-named variables in mapcache xen: add a lock for the mapcache Paul Durrant (2): Add device listener interface Xen: Use the ioreq-server API when available configure | 29 ++ hw/core/qdev.c | 53 ++ include/hw/qdev-core.h | 10 ++ include/hw/xen/xen_common.h | 223 +++ include/qemu/typedefs.h |1 + trace-events|9 ++ xen-hvm.c | 160 ++- xen-mapcache.c | 94 -- 8 files changed, 526 insertions(+), 53 deletions(-)
Re: [Qemu-devel] [PATCH] target-arm/translate-a64: Fix wrong mmu_idx usage for LDT/STT
On Tue, Jan 20, 2015 at 7:58 AM, Peter Maydell peter.mayd...@linaro.org
wrote:
The LDT/STT (load/store unprivileged) instruction decode was using
the wrong MMU index value. This meant that instead of these insns
being always access as if user-mode regardless of current privilege
they were always access as if kernel-mode regardless of current
privilege. This went unnoticed because AArch64 Linux doesn't use
these instructions.
Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
I'm not counting this as a security issue because I'm assuming
nobody treats TCG guests as a security boundary (certainly I
would not recommend doing so...)
---
target-arm/translate-a64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 80d2359..dac2f63 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -2107,7 +2107,7 @@ static void disas_ldst_reg_imm9(DisasContext *s,
uint32_t insn)
}
} else {
TCGv_i64 tcg_rt = cpu_reg(s, rt);
-int memidx = is_unpriv ? 1 : get_mem_index(s);
+int memidx = is_unpriv ? MMU_USER_IDX : get_mem_index(s);
if (is_store) {
do_gpr_st_memidx(s, tcg_rt, tcg_addr, size, memidx);
--
1.9.1
Reviewed-by: Greg Bellows greg.bell...@linaro.org
Re: [Qemu-devel] [PATCH 0/2] qcow2: Add two more unalignment checks
Am 20.01.2015 um 14:49 hat Max Reitz geschrieben: On 2015-01-20 at 05:09, Kevin Wolf wrote: Am 19.01.2015 um 22:09 hat Max Reitz geschrieben: On 2015-01-19 at 16:04, Eric Blake wrote: On 01/19/2015 01:49 PM, Max Reitz wrote: With the series adding unalignment checks and the series reworking the zero cluster expansion code overlapping, the unalignment checks have not been implemented in the latter code. This series fixes it. There are other places which would require unalignment checks, like the offsets of L1 tables, especially for snapshots; but because it would be best to add these checks in the function which reads the snapshot table, this would make images with broken snapshots completely unusable, which is something I opted to avoid for now. That's something I noticed, too: For qemu-img check, our qcow2_open() checks may be to strict. With a corrupted snapshot table, it won't even run. Perhaps we should be laxer with some checks with BDRV_O_CHECK, and for example just set s-nb_snapshots = 0 if loading the table fails. Ideally, we need to make the qcow2 repair function repair such cases, but until that is done there is not much we can do about them. What's the best repair? That's what I was asking myself... Read the data from the unaligned location, and write a fresh copy into a new aligned allocation? Maybe. Maybe there is no way of repairing them and the only way is asking the user whether it's fine to delete the snapshot (qemu-img check -r make_consistent_whatever_it_takes). Also, the question remains for every unaligned data structure: L2 tables, data clusters… The refcount structures are the only ones that can be easily recovered. For data clusters, reading from the unaligned location would probably be best; for L2 tables… maybe the same, then run a consistency check on the data and if it seems off™, throw the whole L2 table away. Reading from the unaligned location doesn't help. I have never seen an image whose L2 entries contained valid entries, except for the least significant few bits. If your table is corrupted, it's usually garbage from start to end. Probably so. I think the only sane option is to drop the entries. The big question is what should be used to replace them. /dev/random of course. There is a chance we are correct… Seriously speaking, well, unmap them? Zero clusters don't feel right to me. Actually, I have a feeling that zero clusters are better because they are obviously wrong when you debug a problem in the guest. Falling back to the backing file may result in patterns that don't look obviously corrupted, or even expose information to users that shouldn't have permission to read it. Kevin
