Re: [Qemu-devel] [PATCH] keyboard: fix qemu load empty keymap
03.11.2016 08:56, Wang Xin wrote:
> qemu_find_file do not check file is a directory or just a file.
> If qemu start with "-k ''", qemu_find_file get a empty string
> as keymap file name, then, qemu treat the keymap path as keymap
> file, it makes vnc keyboard input unusable.
Do we really care? "Garbage in, garbage out" I'd say :)
Thanks,
/mjt
> diff --git a/vl.c b/vl.c
> index ebd47af..2ec3832 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2264,6 +2264,7 @@ char *qemu_find_file(int type, const char *name)
> int i;
> const char *subdir;
> char *buf;
> +struct stat file_stat;
>
> /* Try the name as a straight path first */
> if (access(name, R_OK) == 0) {
> @@ -2284,7 +2285,13 @@ char *qemu_find_file(int type, const char *name)
>
> for (i = 0; i < data_dir_idx; i++) {
> buf = g_strdup_printf("%s/%s%s", data_dir[i], subdir, name);
> -if (access(buf, R_OK) == 0) {
> +if (stat(buf, &file_stat) < 0) {
> +error_report("can not get file '%s' stat: %s\n", buf,
> + strerror(errno));
> +g_free(buf);
> +return NULL;
> +}
> +if (!S_ISDIR(file_stat.st_mode) && access(buf, R_OK) == 0) {
> trace_load_file(name, buf);
> return buf;
> }
>
Re: [Qemu-devel] [PATCH for-2.8?] rules.mak: speedup save-vars load-vars
On Wed, 11/02 17:24, Paolo Bonzini wrote: > Unnesting variables spends a lot of time parsing and executing foreach > and if functions. Because actually very few variables have to be > saved and restored, a good strategy is to remember what has to be done > in load-vars, and only iterate the right variables in load-vars. > For save-vars, unroll the foreach loop to provide another small > improvement. > > This speeds up a "noop" build from around 15.5 seconds on my laptop > to 11.7 (25% roughly). > > Signed-off-by: Paolo Bonzini > --- > I'm wondering if this would be acceptable for 2.8. I think that's fine if you don't want to bear the slowness in 2.8. But TBH I haven't really noticed this as an issue myself. Anyway, Reviewed-by: Fam Zheng > I also have sent patches to GNU make that save another > 20%, down to 9.8 seconds. Wow, is there a link to the patch? Thanks, Fam
Re: [Qemu-devel] [PATCH v5 10/10] msi_init: convert assert to return -errno
Please ignore this one, I forget to commit the amendment...
Already send the right one.
Cao jin
On 11/03/2016 12:06 PM, Cao jin wrote:
According to the disscussion:
http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg08215.html
Let leaf function returns reasonable -errno, let caller decide how to
handle the return value.
Suggested-by: Markus Armbruster
CC: Markus Armbruster
CC: Michael S. Tsirkin
CC: Marcel Apfelbaum
Reviewed-by: Markus Armbruster
Signed-off-by: Cao jin
---
hw/pci/msi.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a87b227..443682b 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -201,9 +201,14 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
" 64bit %d mask %d\n",
offset, nr_vectors, msi64bit, msi_per_vector_mask);
-assert(!(nr_vectors & (nr_vectors - 1))); /* power of 2 */
-assert(nr_vectors > 0);
-assert(nr_vectors <= PCI_MSI_VECTORS_MAX);
+/* vector sanity test: should in range 1 - 32, should be power of 2 */
+if ((nr_vectors == 0) ||
+(nr_vectors > PCI_MSI_VECTORS_MAX) ||
+(nr_vectors & (nr_vectors - 1))) {
+error_setg(errp, "Invalid vector number: %d", nr_vectors);
+return -EINVAL;
+}
+
/* the nr of MSI vectors is up to 32 */
vectors_order = ctz32(nr_vectors);
[Qemu-devel] [PATCH v5 10/10] msi_init: convert assert to return -errno
According to the disscussion:
http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg08215.html
Let leaf function returns reasonable -errno, let caller decide how to
handle the return value.
Suggested-by: Markus Armbruster
CC: Markus Armbruster
CC: Michael S. Tsirkin
CC: Marcel Apfelbaum
Reviewed-by: Markus Armbruster
Signed-off-by: Cao jin
---
hw/pci/msi.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
Really sorry, I forget to commit what I amend...
diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a87b227..af3efbe 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -201,9 +201,12 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
" 64bit %d mask %d\n",
offset, nr_vectors, msi64bit, msi_per_vector_mask);
-assert(!(nr_vectors & (nr_vectors - 1))); /* power of 2 */
-assert(nr_vectors > 0);
-assert(nr_vectors <= PCI_MSI_VECTORS_MAX);
+/* vector sanity test: should in range 1 - 32, should be power of 2 */
+if (!is_power_of_2(nr_vectors) || nr_vectors > PCI_MSI_VECTORS_MAX) {
+error_setg(errp, "Invalid vector number: %d", nr_vectors);
+return -EINVAL;
+}
+
/* the nr of MSI vectors is up to 32 */
vectors_order = ctz32(nr_vectors);
--
2.1.0
Re: [Qemu-devel] [PATCH v5 00/10] Convert msix_init() to error
sorry I forget to cc maintainers in cover letter. On 11/03/2016 12:06 PM, Cao jin wrote: v5 changelog: 1. Address to all comments of Markus 2. Add the given R-b to each commit message. Cao jin (10): msix: Follow CODING_STYLE hcd-xhci: check & correct param before using it pci: Convert msix_init() to Error and fix callers to check it megasas: change behaviour of msix switch hcd-xhci: change behaviour of msix switch megasas: remove unnecessary megasas_use_msix() megasas: undo the overwrites of msi user configuration vmxnet3: fix reference leak issue vmxnet3: remove unnecessary internal msix flag msi_init: convert assert to return -errno hw/block/nvme.c| 5 +++- hw/misc/ivshmem.c | 8 +++--- hw/net/e1000e.c| 6 - hw/net/rocker/rocker.c | 7 - hw/net/vmxnet3.c | 46 ++-- hw/pci/msi.c | 11 +--- hw/pci/msix.c | 42 - hw/scsi/megasas.c | 49 +++--- hw/usb/hcd-xhci.c | 71 ++ hw/vfio/pci.c | 4 ++- hw/virtio/virtio-pci.c | 11 include/hw/pci/msix.h | 5 ++-- 12 files changed, 164 insertions(+), 101 deletions(-) -- Yours Sincerely, Cao jin
[Qemu-devel] [PATCH] keyboard: fix qemu load empty keymap
qemu_find_file do not check file is a directory or just a file.
If qemu start with "-k ''", qemu_find_file get a empty string
as keymap file name, then, qemu treat the keymap path as keymap
file, it makes vnc keyboard input unusable.
Signed-off-by: Wang Xin
diff --git a/vl.c b/vl.c
index ebd47af..2ec3832 100644
--- a/vl.c
+++ b/vl.c
@@ -2264,6 +2264,7 @@ char *qemu_find_file(int type, const char *name)
int i;
const char *subdir;
char *buf;
+struct stat file_stat;
/* Try the name as a straight path first */
if (access(name, R_OK) == 0) {
@@ -2284,7 +2285,13 @@ char *qemu_find_file(int type, const char *name)
for (i = 0; i < data_dir_idx; i++) {
buf = g_strdup_printf("%s/%s%s", data_dir[i], subdir, name);
-if (access(buf, R_OK) == 0) {
+if (stat(buf, &file_stat) < 0) {
+error_report("can not get file '%s' stat: %s\n", buf,
+ strerror(errno));
+g_free(buf);
+return NULL;
+}
+if (!S_ISDIR(file_stat.st_mode) && access(buf, R_OK) == 0) {
trace_load_file(name, buf);
return buf;
}
--
2.8.1.windows.1
[Qemu-devel] 答复： [PATCH v4 5/5] object: Add 'help' option for all available backends and properties
ping...
>>> Lin Ma 2016/10/20 星期四 下午 7:28 >>>
'-object help' prints available user creatable backends.
'-object $typename,help' prints relevant properties.
Signed-off-by: Lin Ma
---
include/qom/object_interfaces.h | 2 ++
qemu-options.hx | 7 +-
qom/object_interfaces.c | 55 +
vl.c| 5
4 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h
index 8b17f4d..197cd5f 100644
--- a/include/qom/object_interfaces.h
+++ b/include/qom/object_interfaces.h
@@ -165,4 +165,6 @@ int user_creatable_add_opts_foreach(void *opaque,
*/
void user_creatable_del(const char *id, Error **errp);
+int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp);
+
#endif
diff --git a/qemu-options.hx b/qemu-options.hx
index b1fbdb0..b9573ce 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -3761,7 +3761,9 @@ DEF("object", HAS_ARG, QEMU_OPTION_object,
" create a new object of type TYPENAME
setting properties\n"
" in the order they are specified. Note that
the 'id'\n"
" property must be set. These objects are
placed in the\n"
-" '/objects' path.\n",
+" '/objects' path.\n"
+" Use '-object help' to print available backend types
and\n"
+" '-object typename,help' to print relevant
properties.\n",
QEMU_ARCH_ALL)
STEXI
@item -object @var{typename}[,@var{prop1}=@var{value1},...]
@@ -3771,6 +3773,9 @@ in the order they are specified. Note that the 'id'
property must be set. These objects are placed in the
'/objects' path.
+Use @code{-object help} to print available backend types and
+@code{-object @var{typename},help} to print relevant properties.
+
@table @option
@item -object
memory-backend-file,id=@var{id},size=@var{size},mem-path=@var{dir},share=@var{on|off}
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index bf59846..da8be39 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -5,6 +5,7 @@
#include "qapi-visit.h"
#include "qapi/qmp-output-visitor.h"
#include "qapi/opts-visitor.h"
+#include "qemu/help_option.h"
void user_creatable_complete(Object *obj, Error **errp)
{
@@ -212,6 +213,60 @@ void user_creatable_del(const char *id, Error **errp)
object_unparent(obj);
}
+int user_creatable_help_func(void *opaque, QemuOpts *opts, Error **errp)
+{
+const char *type = NULL;
+Object *obj = NULL;
+ObjectClass *klass;
+ObjectProperty *prop;
+ObjectPropertyIterator iter;
+
+type = qemu_opt_get(opts, "qom-type");
+if (type && is_help_option(type)) {
+ GSList *list;
+ printf("Available object backend types:\n");
+ for (list = object_class_get_list(TYPE_USER_CREATABLE, false); \
+ list;
\
+ list = list->next) {
+ const char *name;
+ name = object_class_get_name(OBJECT_CLASS(list->data));
+ if (strcmp(name, TYPE_USER_CREATABLE)) {
+ printf("%s\n", name);
+ }
+ }
+ g_slist_free(list);
+ goto out;
+}
+
+if (!type || !qemu_opt_has_help_opt(opts)) {
+ return 0;
+}
+
+klass = object_class_by_name(type);
+if (!klass) {
+ printf("invalid object type: %s\n", type);
+ goto out;
+}
+if (object_class_is_abstract(klass)) {
+ printf("object type '%s' is abstract\n", type);
+ goto out;
+}
+obj = object_new(type);
+object_property_iter_init(&iter, obj);
+
+while ((prop = object_property_iter_next(&iter))) {
+ if (prop->description) {
+ printf("%s (%s, %s)\n", prop->name, prop->type,
prop->description);
+ } else {
+ printf("%s (%s)\n", prop->name, prop->type);
+ }
+}
+
+out:
+object_unref(obj);
+return 1;
+}
+
static void register_types(void)
{
static const TypeInfo uc_interface_info = {
diff --git a/vl.c b/vl.c
index ebd47af..145
6eca 100644
--- a/vl.c
+++ b/vl.c
@@ -4100,6 +4100,11 @@ int main(int argc, char **argv, char **envp)
exit(0);
}
+if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_help_func,
+ NULL, NULL)) {
+ exit(1);
+}
+
if (!trace_init_backends()) {
exit(1);
}
--
2.9.2
[Qemu-devel] 答复： [PATCH v4 3/5] qapi: add test case for the generated enum value str
ping...
>>> Lin Ma 2016/10/20 星期四 下午 7:28 >>>
Signed-off-by: Lin Ma
---
tests/test-qmp-commands.c | 18 ++
1 file changed, 18 insertions(+)
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index 81cbe54..9cd61b2 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -262,6 +262,23 @@ static void test_dealloc_partial(void)
qapi_free_UserDefTwo(ud2);
}
+/* test generated enum value str */
+static void test_enum_value_str(void)
+{
+EnumOne i;
+char *expected_str = NULL;
+
+for (i = 0; i < ENUM_ONE__MAX; i++) {
+ if (i == 0) {
+ expected_str = g_strdup_printf("\'%s\'", EnumOne_lookup[i]);
+ } else {
+ expected_str = g_strdup_printf("%s, \'%s\'",
+
expected_str, EnumOne_lookup[i]);
+ }
+}
+g_assert_cmpstr(EnumOne_value_str, ==, expected_str);
+}
+
int main(int argc, char **argv)
{
@@ -272,6 +289,7 @@ int main(int argc, char **argv)
g_test_add_func("/0.15/dispatch_cmd_io", test_dispatch_cmd_io);
g_test_add_func("/0.15/dealloc_types", test_dealloc_types);
g_test_add_func("/0.15/dealloc_partial", test_dealloc_partial);
+g_test_add_func("/0.15/enum_value_str", test_enum_value_str);
module_call_init(MODULE_INIT_QAPI);
g_test_run();
--
2.9.2
[Qemu-devel] 答复： [PATCH v4 2/5] qapi: auto generate enum value strings
ping...
>>> Lin Ma 2016/10/20 星期四 下午 7:28 >>>
Automatically generate enum value strings that containing the acceptable values.
(Borrowed Daniel's code.)
Signed-off-by: Lin Ma
---
scripts/qapi-types.py | 2 ++
scripts/qapi.py| 9 +
2 files changed, 11 insertions(+)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index dabc42e..0446839 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -202,9 +202,11 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
self._btin += gen_enum(name, values, prefix)
if do_builtins:
self.defn += gen_enum_lookup(name, values,
prefix)
+ self._btin += gen_enum_value_str(name, values)
else:
self._fwdecl += gen_enum(name, values, prefix)
self.defn += gen_enum_lookup(name, values, prefix)
+ self._fwdecl += gen_enum_value_str(name, values)
def visit_array_type(self, name, info, element_type):
if isinstance(element_type, QAPISchemaBuiltinType):
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 21bc32f..d11c414 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1649,6 +1649,15 @@ const char *const %(c_name)s_lookup[] = {
return ret
+def gen_enum_value_str(name, values):
+return mcgen('''
+
+#define %(c_name)s_value_str "%(value_str)s"
+''',
+ c_name=c_name(name),
+ value_str=", ".join(["'%s'" % c for c in values]))
+
+
def gen_enum(name, values, prefix=None):
# append automatically generated _MAX value
enum_values = values + ['_MAX']
--
2.9.2
[Qemu-devel] 答复： [PATCH v4 4/5] backends: add description for enum class properties
ping...
>>> Lin Ma 2016/10/20 星期四 下午 7:28 >>>
Signed-off-by: Lin Ma
---
backends/hostmem.c | 4
crypto/secret.c| 4
crypto/tlscreds.c | 4
net/filter.c | 4
4 files changed, 16 insertions(+)
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 4256d24..25f303d 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -377,6 +377,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
HostMemPolicy_lookup,
host_memory_backend_get_policy,
host_memory_backend_set_policy, &error_abort);
+object_class_property_set_description(oc, "policy",
+ "Data
format: one of "
+
HostMemPolicy_value_str,
+
&error_abort);
}
static const TypeInfo host_memory_backend_info = {
diff --git a/crypto/secret.c b/crypto/secret.c
index 285ab7a..71d06e3 100644
--- a/crypto/secret.c
+++ b/crypto/secret.c
@@ -382,6 +382,10 @@ qcrypto_secret_class_init(ObjectClass *oc, void *data)
qcrypto_secret_prop_get_format,
qcrypto_secret_prop_set_format,
NULL);
+object_class_property_set_description(oc, "format",
+
"Data format: one of "
+
QCryptoSecretFormat_value_str,
+
&error_abort);
object_class_property_add_str(oc, "data",
qcrypto_secret_prop_get_data,
qcrypto_secret_prop_set_data,
diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c
index a896553..d3af38a 100644
--- a/crypto/tlscreds.c
+++ b/crypto/tlscreds.c
@@ -237,6 +237,10 @@ qcrypto_tls_creds_class_init(ObjectClass *oc, void *data)
qcrypto_tls_creds_prop_get_endpoint,
qcrypto_tls_creds_prop_set_endpoint,
NULL);
+object_class_property_set_description(oc, "endpoint",
+
"Data format: one of "
+
QCryptoTLSCredsEndpoint_value_str,
+
&error_abort);
object_class_property_add_str(oc, "priority",
qcrypto_tls_creds_prop_get_priority,
qcrypto_tls_creds_prop_set_priority,
diff --git a/net/filter.c b/net/filter.c
index 1dfd2ca..205fb49 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -182,6 +182,10 @@ static void netfilter_init(Object *obj)
NetFilterDirection_lookup,
netfilter_get_direction, netfilter_set_direction,
NULL);
+object_property_set_description(obj, "queue",
+ "Data
format: one of "
+
NetFilterDirection_value_str,
+
&error_abort);
object_property_add_str(obj, "status",
netfilter_get_status,
netfilter_set_status,
NULL);
--
2.9.2
[Qemu-devel] 答复： [PATCH v4 0/5] object: Add 'help' option for all available backends and properties
ping... >>> Lin Ma 2016/10/20 星期四 下午 7:28 >>> V3->V4: * drop the code about manually define interface 'user-creatable' as abstract. * add test case for the generated enum value str. * split the code about adding descriptions to class properties into a separate patch. * minor changes in user_creatable_help_func to ignore interface TYPE_USER_CREATABLE. V2->v3: * make type user-creatable abstract. * auto generate enum value strings during qemu configuration.(Borrowwed Daniel's code) * save the generated enum value strings into member description of ObjectProperty. * drop the judgement logic of whether a property has an enumeration type anymore, output member description of ObjectProperty directly. * at least, user_creatable_help_func should be put after 'object_property_add_child(object_get_root(), "machine",OBJECT(current_machine), ...)', because host_memory_backend_init needs to access an instance of type machine. V1->V2: * Output the acceptable values of enum types by "-object TYPE-NAME,help" Lin Ma (5): qom: Add interface check in object_class_is_abstract qapi: auto generate enum value strings qapi: add test case for the generated enum value str backends: add description for enum class properties object: Add 'help' option for all available backends and properties backends/hostmem.c| 4 +++ crypto/secret.c | 4 +++ crypto/tlscreds.c | 4 +++ include/qom/object_interfaces.h | 2 ++ net/filter.c| 4 +++ qemu-options.hx | 7 +- qom/object.c| 6 - qom/object_interfaces.c | 55 + scripts/qapi-types.py | 2 ++ scripts/qapi.py | 9 +++ tests/test-qmp-commands.c | 18 ++ vl.c| 5 12 files changed, 118 insertions(+), 2 deletions(-) -- 2.9.2
[Qemu-devel] 答复： [PATCH v4 1/5] qom: Add interface check in object_class_is_abstract
ping...
>>> Lin Ma 2016/10/20 星期四 下午 7:28 >>>
Signed-off-by: Lin Ma
---
qom/object.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/qom/object.c b/qom/object.c
index 7a05e35..4096645 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -747,7 +747,11 @@ ObjectClass *object_get_class(Object *obj)
bool object_class_is_abstract(ObjectClass *klass)
{
-return klass->type->abstract;
+if (type_is_ancestor(klass->type, type_interface)) {
+ return true;
+} else {
+ return klass->type->abstract;
+}
}
const char *object_class_get_name(ObjectClass *klass)
--
2.9.2
Re: [Qemu-devel] [PATCH v3 0/4] nvdimm: hotplug support
On Wed, Nov 02, 2016 at 03:01:33PM +0100, Igor Mammedov wrote: > On Sat, 29 Oct 2016 00:35:36 +0800 > Xiao Guangrong wrote: > > > It is based on my previous patchset, > > "[PATCH 0/8] nvdimm acpi: bug fix and cleanup", these two patchset are > > against commit dea651a95af6dad099 (intel-iommu: Check IOAPIC's Trigger Mode > > against the one in IRTE) on pci branch of Michael's git tree and can be > > found at: > > https://github.com/xiaogr/qemu.git nvdimm-hotplug-v3 > > > > Changelog in v3: > >1) use a dedicated interrupt for nvdimm device hotplug > >2) stop nvdimm device hot unplug > >3) reserve UUID and handle for QEMU internally used QEMU > >5) redesign fit buffer to avoid OSPM reading incomplete fit info > >6) bug fixes and cleanups > > > > Changelog in v2: > >Fixed signed integer overflow pointed out by Stefan Hajnoczi > > > > This patchset enables nvdimm hotplug support, it is used as pc-dimm hotplug, > > for example, a new nvdimm device can be plugged as follows: > > object_add memory-backend-file,id=mem3,size=10G,mem-path=/home/eric/nvdimm3 > > device_add nvdimm,id=nvdimm3,memdev=mem3 > > > > and unplug it as follows: > > device_del nvdimm3 > > object_del mem3 > there is no unplug support > > > instead of incremental fixups on top merged patches in followup series, > I'd prefer it to make a clean revert for patches 2-4/4 first and > them amended versions of them to follow. Let's get the fixes reviewed first, how to apply them is a minor issue in my eyes. > > > > Xiao Guangrong (4): > > nvdimm acpi: prebuild nvdimm devices for available slots > > nvdimm acpi: introduce fit buffer > > nvdimm acpi: introduce _FIT > > pc: memhp: enable nvdimm device hotplug > > > > docs/specs/acpi_mem_hotplug.txt | 3 + > > docs/specs/acpi_nvdimm.txt | 58 ++- > > hw/acpi/memory_hotplug.c | 31 +++- > > hw/acpi/nvdimm.c | 286 > > +++ > > hw/core/hotplug.c| 11 ++ > > hw/core/qdev.c | 20 ++- > > hw/i386/acpi-build.c | 9 +- > > hw/i386/pc.c | 31 > > hw/mem/nvdimm.c | 4 - > > include/hw/acpi/acpi_dev_interface.h | 1 + > > include/hw/hotplug.h | 10 ++ > > include/hw/mem/nvdimm.h | 27 +++- > > 12 files changed, 443 insertions(+), 48 deletions(-) > >
Re: [Qemu-devel] [PATCH v4 0/3] nvdimm: hotplug support
On Thu, Nov 03, 2016 at 12:25:01PM +0800, Xiao Guangrong wrote:
>
>
> On 11/03/2016 12:14 PM, Michael S. Tsirkin wrote:
> > On Thu, Nov 03, 2016 at 11:51:27AM +0800, Xiao Guangrong wrote:
> > > Resend these 3 patches to catch up release window...
> > >
> > > Igor,
> > >
> > > this is a open that i did not pass a buffer as parameter to RFIT as
> > > tried the way you suggested, but failed. May be i am not very good at
> > > ASL, i need more time to try. So let's keep the way as it is, i will
> > > improve it later.
> > >
> > > Thanks!
> >
> > And just for comparison, could you pls generate the
> > incremental diff as well?
>
> Hi Michael,
>
> Okay. This is the diff comparing with v3 and v4:
So I don't see how all this requires revert and reapply
and IMHO review of incremental patches would be easier
but if it makes Igor happier, so be it.
I'll wait until we have acks though.
> diff --git a/default-configs/mips-softmmu-common.mak
> b/default-configs/mips-softmmu-common.mak
> index 0394514..f0676f5 100644
> --- a/default-configs/mips-softmmu-common.mak
> +++ b/default-configs/mips-softmmu-common.mak
> @@ -17,6 +17,7 @@ CONFIG_FDC=y
> CONFIG_ACPI=y
> CONFIG_ACPI_X86=y
> CONFIG_ACPI_MEMORY_HOTPLUG=y
> +CONFIG_ACPI_NVDIMM=y
> CONFIG_ACPI_CPU_HOTPLUG=y
> CONFIG_APM=y
> CONFIG_I8257=y
> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
> index cb26dd2..3df3620 100644
> --- a/docs/specs/acpi_mem_hotplug.txt
> +++ b/docs/specs/acpi_mem_hotplug.txt
> @@ -4,9 +4,6 @@ QEMU<->ACPI BIOS memory hotplug interface
> ACPI BIOS GPE.3 handler is dedicated for notifying OS about memory hot-add
> and hot-remove events.
>
> -ACPI BIOS GPE.4 handler is dedicated for notifying OS about nvdimm device
> -hot-add and hot-remove events.
> -
> Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
> ---
> 0xa00:
> diff --git a/docs/specs/acpi_nvdimm.txt b/docs/specs/acpi_nvdimm.txt
> index 4aa5e3d..7887e57 100644
> --- a/docs/specs/acpi_nvdimm.txt
> +++ b/docs/specs/acpi_nvdimm.txt
> @@ -65,8 +65,8 @@ _FIT(Firmware Interface Table)
> The detailed definition of the structure can be found at ACPI 6.0: 5.2.25
> NVDIMM Firmware Interface Table (NFIT).
>
> -QEMU NVDIMM Implemention
> -
> +QEMU NVDIMM Implementation
> +==
> QEMU uses 4 bytes IO Port starting from 0x0a18 and a RAM-based memory page
> for NVDIMM ACPI.
>
> @@ -82,6 +82,16 @@ Memory:
> ACPI writes _DSM Input Data (based on the offset in the page):
> [0x0 - 0x3]: 4 bytes, NVDIMM Device Handle, 0 is reserved for NVDIMM
> Root device.
> +
> +The handle is completely QEMU internal thing, the values in
> +range [0, 0x] indicate nvdimm device (O means nvdimm
> +root device named NVDR), other values are reserved by other
> +purpose.
> +
> +Current reserved handle:
> +0x1 is reserved for QEMU internal DSM function called on
> +the root device.
> +
> [0x4 - 0x7]: 4 bytes, Revision ID, that is the Arg1 of _DSM method.
> [0x8 - 0xB]: 4 bytes. Function Index, that is the Arg2 of _DSM method.
> [0xC - 0xFFF]: 4084 bytes, the Arg3 of _DSM method.
> @@ -127,28 +137,22 @@ _DSM process diagram:
> | result from the page | | |
> +--+ +--+
>
> -Device Handle Reservation
> --
> -As we mentioned above, byte 0 ~ byte 3 in the DSM memory save NVDIMM device
> -handle. The handle is completely QEMU internal thing, the values in range
> -[0, 0x] indicate nvdimm device (O means nvdimm root device named NVDR),
> -other values are reserved by other purpose.
> -
> -Current reserved handle:
> -0x1 is reserved for QEMU internal DSM function called on the root
> -device.
> +NVDIMM hotplug
> +--
> +ACPI BIOS GPE.4 handler is dedicated for notifying OS about nvdimm device
> +hot-add and hot-remove events.
>
> QEMU internal use only _DSM function
>
> -UUID, 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62, is reserved for QEMU internal
> -DSM function.
> -
> There is the function introduced by QEMU and only used by QEMU internal.
>
> 1) Read FIT
> - As we only reserved one page for NVDIMM ACPI it is impossible to map the
> - whole FIT data to guest's address space. This function is used by _FIT
> - method to read a piece of FIT data from QEMU.
> + UUID 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62 is reserved for Read_FIT DSM
> + function (private QEMU function)
> +
> + _FIT method uses Read_FIT function to fetch NFIT structures blob from
> + QEMU in 1 page sized increments which are then concatenated and returned
> + as _FIT method result.
>
> Input parameters:
> Arg0 – UUID {set to 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62}
>
Re: [Qemu-devel] [kvm-unit-tests PATCHv7 2/3] arm: pmu: Check cycle count increases
Hi Wei,
Thanks for your work on this.
On 2016-11-02 16:22, Wei Huang wrote:
Ensure that reads of the PMCCNTR_EL0 are monotonically increasing,
even for the smallest delta of two subsequent reads.
Signed-off-by: Christopher Covington
Signed-off-by: Wei Huang
---
arm/pmu.c | 100
++
1 file changed, 100 insertions(+)
diff --git a/arm/pmu.c b/arm/pmu.c
index 42d0ee1..65b7df1 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -14,6 +14,9 @@
*/
#include "libcflat.h"
+#define NR_SAMPLES 10
+#define ARMV8_PMU_CYCLE_IDX 31
+
#if defined(__arm__)
static inline uint32_t get_pmcr(void)
{
@@ -22,6 +25,43 @@ static inline uint32_t get_pmcr(void)
asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret));
return ret;
}
+
+static inline void set_pmcr(uint32_t pmcr)
+{
+ asm volatile("mcr p15, 0, %0, c9, c12, 0" : : "r" (pmcr));
+}
+
+static inline void set_pmccfiltr(uint32_t filter)
+{
+ uint32_t cycle_idx = ARMV8_PMU_CYCLE_IDX;
+
+ asm volatile("mcr p15, 0, %0, c9, c12, 5" : : "r" (cycle_idx));
+ asm volatile("mcr p15, 0, %0, c9, c13, 1" : : "r" (filter));
+}
Down the road I'd like to add tests for the regular events. What if you
added
separate PMSELR and PMXEVTYPER accessors now and used them (with
PMSELR.SEL = 31)
for setting PMCCFILTR? Then we wouldn't need a specialized set_pmccfiltr
function for the cycle counter versus PMSELR and PMXEVTYPER for the
regular events.
+/*
+ * While PMCCNTR can be accessed as a 64 bit coprocessor register,
returning 64
+ * bits doesn't seem worth the trouble when differential usage of the
result is
+ * expected (with differences that can easily fit in 32 bits). So just
return
+ * the lower 32 bits of the cycle count in AArch32.
+ */
+static inline unsigned long get_pmccntr(void)
+{
+ unsigned long cycles;
+
+ asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
+ return cycles;
+}
+
+static inline void enable_counter(uint32_t idx)
+{
+ asm volatile("mcr p15, 0, %0, c9, c12, 1" : : "r" (1 << idx));
+}
My personal preference, that I think would make this function look and
act like the other system register accessor functions, would be to
name the function "set_pmcntenset" and do a plain write of the input
parameter without a shift, letting the shift be done in the C code.
(As we scale up, the system register accessor functions should probably
be generated by macros from a concise table.)
+static inline void disable_counter(uint32_t idx)
+{
+ asm volatile("mrc p15, 0, %0, c9, c12, 1" : : "r" (1 << idx));
+}
This function doesn't seem to be used yet. Consider whether it might
make sense to defer introducing it until there is a user.
#elif defined(__aarch64__)
static inline uint32_t get_pmcr(void)
{
@@ -30,6 +70,34 @@ static inline uint32_t get_pmcr(void)
asm volatile("mrs %0, pmcr_el0" : "=r" (ret));
return ret;
}
+
+static inline void set_pmcr(uint32_t pmcr)
+{
+ asm volatile("msr pmcr_el0, %0" : : "r" (pmcr));
+}
+static inline void set_pmccfiltr(uint32_t filter)
+{
+ asm volatile("msr pmccfiltr_el0, %0" : : "r" (filter));
+}
As above, consider whether using PMSELR and PMXEVTYPER might be a more
reusable pair of accessors.
+static inline unsigned long get_pmccntr(void)
+{
+ unsigned long cycles;
+
+ asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
+ return cycles;
+}
+
+static inline void enable_counter(uint32_t idx)
+{
+ asm volatile("msr pmcntenset_el0, %0" : : "r" (1 << idx));
+}
Same thought as above about uniformity and generatability.
+static inline void disable_counter(uint32_t idx)
+{
+ asm volatile("msr pmcntensclr_el0, %0" : : "r" (1 << idx));
+}
As above, this function doesn't seem to be used yet.
Thanks,
Cov
Re: [Qemu-devel] [PATCH v4 0/3] nvdimm: hotplug support
On 11/03/2016 12:14 PM, Michael S. Tsirkin wrote:
On Thu, Nov 03, 2016 at 11:51:27AM +0800, Xiao Guangrong wrote:
Resend these 3 patches to catch up release window...
Igor,
this is a open that i did not pass a buffer as parameter to RFIT as
tried the way you suggested, but failed. May be i am not very good at
ASL, i need more time to try. So let's keep the way as it is, i will
improve it later.
Thanks!
And just for comparison, could you pls generate the
incremental diff as well?
Hi Michael,
Okay. This is the diff comparing with v3 and v4:
diff --git a/default-configs/mips-softmmu-common.mak
b/default-configs/mips-softmmu-common.mak
index 0394514..f0676f5 100644
--- a/default-configs/mips-softmmu-common.mak
+++ b/default-configs/mips-softmmu-common.mak
@@ -17,6 +17,7 @@ CONFIG_FDC=y
CONFIG_ACPI=y
CONFIG_ACPI_X86=y
CONFIG_ACPI_MEMORY_HOTPLUG=y
+CONFIG_ACPI_NVDIMM=y
CONFIG_ACPI_CPU_HOTPLUG=y
CONFIG_APM=y
CONFIG_I8257=y
diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
index cb26dd2..3df3620 100644
--- a/docs/specs/acpi_mem_hotplug.txt
+++ b/docs/specs/acpi_mem_hotplug.txt
@@ -4,9 +4,6 @@ QEMU<->ACPI BIOS memory hotplug interface
ACPI BIOS GPE.3 handler is dedicated for notifying OS about memory hot-add
and hot-remove events.
-ACPI BIOS GPE.4 handler is dedicated for notifying OS about nvdimm device
-hot-add and hot-remove events.
-
Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
---
0xa00:
diff --git a/docs/specs/acpi_nvdimm.txt b/docs/specs/acpi_nvdimm.txt
index 4aa5e3d..7887e57 100644
--- a/docs/specs/acpi_nvdimm.txt
+++ b/docs/specs/acpi_nvdimm.txt
@@ -65,8 +65,8 @@ _FIT(Firmware Interface Table)
The detailed definition of the structure can be found at ACPI 6.0: 5.2.25
NVDIMM Firmware Interface Table (NFIT).
-QEMU NVDIMM Implemention
-
+QEMU NVDIMM Implementation
+==
QEMU uses 4 bytes IO Port starting from 0x0a18 and a RAM-based memory page
for NVDIMM ACPI.
@@ -82,6 +82,16 @@ Memory:
ACPI writes _DSM Input Data (based on the offset in the page):
[0x0 - 0x3]: 4 bytes, NVDIMM Device Handle, 0 is reserved for NVDIMM
Root device.
+
+The handle is completely QEMU internal thing, the values in
+range [0, 0x] indicate nvdimm device (O means nvdimm
+root device named NVDR), other values are reserved by other
+purpose.
+
+Current reserved handle:
+0x1 is reserved for QEMU internal DSM function called on
+the root device.
+
[0x4 - 0x7]: 4 bytes, Revision ID, that is the Arg1 of _DSM method.
[0x8 - 0xB]: 4 bytes. Function Index, that is the Arg2 of _DSM method.
[0xC - 0xFFF]: 4084 bytes, the Arg3 of _DSM method.
@@ -127,28 +137,22 @@ _DSM process diagram:
| result from the page | | |
+--+ +--+
-Device Handle Reservation
--
-As we mentioned above, byte 0 ~ byte 3 in the DSM memory save NVDIMM device
-handle. The handle is completely QEMU internal thing, the values in range
-[0, 0x] indicate nvdimm device (O means nvdimm root device named NVDR),
-other values are reserved by other purpose.
-
-Current reserved handle:
-0x1 is reserved for QEMU internal DSM function called on the root
-device.
+NVDIMM hotplug
+--
+ACPI BIOS GPE.4 handler is dedicated for notifying OS about nvdimm device
+hot-add and hot-remove events.
QEMU internal use only _DSM function
-UUID, 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62, is reserved for QEMU internal
-DSM function.
-
There is the function introduced by QEMU and only used by QEMU internal.
1) Read FIT
- As we only reserved one page for NVDIMM ACPI it is impossible to map the
- whole FIT data to guest's address space. This function is used by _FIT
- method to read a piece of FIT data from QEMU.
+ UUID 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62 is reserved for Read_FIT DSM
+ function (private QEMU function)
+
+ _FIT method uses Read_FIT function to fetch NFIT structures blob from
+ QEMU in 1 page sized increments which are then concatenated and returned
+ as _FIT method result.
Input parameters:
Arg0 – UUID {set to 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62}
@@ -156,27 +160,29 @@ There is the function introduced by QEMU and only used by
QEMU internal.
Arg2 - Function Index, 0x1
Arg3 - A package containing a buffer whose layout is as follows:
- +--+-+-+---+
- | Filed | Byte Length | Byte Offset | Description |
- +--+-+-+---+
- | offset | 4 |0| the offset of FIT buffer |
- +--+--
Re: [Qemu-devel] [PATCH v9 04/12] vfio iommu: Add support for mediated devices
On 03/11/16 00:18, Kirti Wankhede wrote: > > > On 11/2/2016 6:30 PM, Jike Song wrote: >> On 11/02/2016 08:41 PM, Kirti Wankhede wrote: >>> On 11/2/2016 5:51 PM, Jike Song wrote: On 11/02/2016 12:09 PM, Alexey Kardashevskiy wrote: > Or you could just reference and use @mm as KVM and others do. Or there is > anything else you need from @current than just @mm? > I agree. If @mm is the only thing needed, there is really no reason to refer to the @task :-) >>> >>> In vfio_lock_acct(), that is for page accounting, if mm->mmap_sem is >>> already held then page accounting is deferred, where task structure is >>> used to get mm and work is deferred only if mm exist: >>> mm = get_task_mm(task); get_task_mm() increments mm_users which is basically a number of userspaces holding the reference to mm. As this case it is not a userspace, mm_count needs to be incremented imho. >>> >>> That is where this module need task structure. >> >> Kirti, >> >> By calling get_task_mm you hold a ref on @mm and save it in iommu, >> whenever you want to do something like vfio_lock_acct(), use that mm >> (as you said, if mmap_sem not accessible then defer it to a work, but >> still @mm is the whole information), and put it after the usage. >> >> I still can't see any reason that the @task have to be saved. It's >> always the @mm all the time. Did I miss anything? >> > > If the process is terminated by SIGKILL, as Alexey mentioned in this > mail thread earlier exit_mm() is called first and then all files are > closed. From exit_mm(), task->mm is set to NULL. So from teardown path, > we should call get_task_mm(task) ... which will return NULL, no? > to get current status intsead of using > stale pointer. If you increment either mm_users or mm_count at the exact place where you want to cache task pointer, why would mm pointer become stale until you do mmdrop() or mmput()? -- Alexey signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] hw/pci: disable pci-bridge's shpc by default
On Wed, Nov 02, 2016 at 05:16:42PM +0200, Marcel Apfelbaum wrote:
> The shpc component is optional while ACPI hotplug is used
> for hot-plugging PCI devices into a PCI-PCI bridge.
> Disabling the shpc by default will make slot 0 usable at boot time
at the cost of breaking all hotplug for all non-acpi users.
> and not only for hot-plug, without loosing any functionality.
> Older machines will have shpc enabled for compatibility reasons.
>
> Signed-off-by: Marcel Apfelbaum
Is an extra slot such a big deal? You can always add more bridges ...
> ---
> hw/pci-bridge/pci_bridge_dev.c | 2 +-
> include/hw/compat.h| 4
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
> index 5dbd933..647ad80 100644
> --- a/hw/pci-bridge/pci_bridge_dev.c
> +++ b/hw/pci-bridge/pci_bridge_dev.c
> @@ -163,7 +163,7 @@ static Property pci_bridge_dev_properties[] = {
> DEFINE_PROP_ON_OFF_AUTO(PCI_BRIDGE_DEV_PROP_MSI, PCIBridgeDev, msi,
> ON_OFF_AUTO_AUTO),
> DEFINE_PROP_BIT(PCI_BRIDGE_DEV_PROP_SHPC, PCIBridgeDev, flags,
> -PCI_BRIDGE_DEV_F_SHPC_REQ, true),
> +PCI_BRIDGE_DEV_F_SHPC_REQ, false),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 0f06e11..388b7ec 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -18,6 +18,10 @@
> .driver = "intel-iommu",\
> .property = "x-buggy-eim",\
> .value= "true",\
> +},{\
> +.driver = "pci-bridge",\
> +.property = "shpc",\
> +.value= "on",\
> },
>
> #define HW_COMPAT_2_6 \
> --
> 2.5.5
Re: [Qemu-devel] [PATCH v4 0/3] nvdimm: hotplug support
On Thu, Nov 03, 2016 at 11:51:27AM +0800, Xiao Guangrong wrote: > Resend these 3 patches to catch up release window... > > Igor, > > this is a open that i did not pass a buffer as parameter to RFIT as > tried the way you suggested, but failed. May be i am not very good at > ASL, i need more time to try. So let's keep the way as it is, i will > improve it later. > > Thanks! And just for comparison, could you pls generate the incremental diff as well? > Changelog in v4: >1) drop fit lock and post_hotplug_cb >2) move nvdimm hotplug code to hw/acpi/nvdimm.c >3) introduce length field to indicate the fit size >4) nvdimm acpi cleanup >5) doc typo fixes > > Changelog in v3: >1) use a dedicated interrupt for nvdimm device hotplug >2) stop nvdimm device hot unplug >3) reserve UUID and handle for QEMU internally used QEMU >5) redesign fit buffer to avoid OSPM reading incomplete fit info >6) bug fixes and cleanups > > Changelog in v2: >Fixed signed integer overflow pointed out by Stefan Hajnoczi > > This patchset enables nvdimm hotplug support, it is used as pc-dimm hotplug, > for example, a new nvdimm device can be plugged as follows: > object_add memory-backend-file,id=mem3,size=10G,mem-path=/home/eric/nvdimm3 > device_add nvdimm,id=nvdimm3,memdev=mem3 > > and unplug it as follows: > device_del nvdimm3 > > Xiao Guangrong (3): > nvdimm acpi: introduce fit buffer > nvdimm acpi: introduce _FIT > pc: memhp: enable nvdimm device hotplug > > default-configs/mips-softmmu-common.mak | 1 + > docs/specs/acpi_nvdimm.txt | 68 +++- > hw/acpi/ich9.c | 8 +- > hw/acpi/nvdimm.c| 289 > > hw/acpi/piix4.c | 7 +- > hw/i386/acpi-build.c| 9 +- > hw/i386/pc.c| 16 ++ > hw/mem/nvdimm.c | 4 - > include/hw/acpi/acpi_dev_interface.h| 1 + > include/hw/mem/nvdimm.h | 22 ++- > 10 files changed, 377 insertions(+), 48 deletions(-) > > -- > 1.8.3.1
[Qemu-devel] [PATCH v4 3/3] pc: memhp: enable nvdimm device hotplug
_GPE.E04 is dedicated for nvdimm device hotplug
Signed-off-by: Xiao Guangrong
---
default-configs/mips-softmmu-common.mak | 1 +
docs/specs/acpi_nvdimm.txt | 5 +
hw/acpi/ich9.c | 8 ++--
hw/acpi/nvdimm.c| 7 +++
hw/acpi/piix4.c | 7 ++-
hw/i386/acpi-build.c| 7 +++
hw/i386/pc.c| 12
hw/mem/nvdimm.c | 4
include/hw/acpi/acpi_dev_interface.h| 1 +
include/hw/mem/nvdimm.h | 1 +
10 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/default-configs/mips-softmmu-common.mak
b/default-configs/mips-softmmu-common.mak
index 0394514..f0676f5 100644
--- a/default-configs/mips-softmmu-common.mak
+++ b/default-configs/mips-softmmu-common.mak
@@ -17,6 +17,7 @@ CONFIG_FDC=y
CONFIG_ACPI=y
CONFIG_ACPI_X86=y
CONFIG_ACPI_MEMORY_HOTPLUG=y
+CONFIG_ACPI_NVDIMM=y
CONFIG_ACPI_CPU_HOTPLUG=y
CONFIG_APM=y
CONFIG_I8257=y
diff --git a/docs/specs/acpi_nvdimm.txt b/docs/specs/acpi_nvdimm.txt
index 364e832..7887e57 100644
--- a/docs/specs/acpi_nvdimm.txt
+++ b/docs/specs/acpi_nvdimm.txt
@@ -137,6 +137,11 @@ _DSM process diagram:
| result from the page | | |
+--+ +--+
+NVDIMM hotplug
+--
+ACPI BIOS GPE.4 handler is dedicated for notifying OS about nvdimm device
+hot-add and hot-remove events.
+
QEMU internal use only _DSM function
There is the function introduced by QEMU and only used by QEMU internal.
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index e5a3c18..830c475 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -490,8 +490,12 @@ void ich9_pm_device_plug_cb(HotplugHandler *hotplug_dev,
DeviceState *dev,
if (lpc->pm.acpi_memory_hotplug.is_enabled &&
object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
-acpi_memory_plug_cb(hotplug_dev, &lpc->pm.acpi_memory_hotplug,
-dev, errp);
+if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
+nvdimm_acpi_plug_cb(hotplug_dev, dev);
+} else {
+acpi_memory_plug_cb(hotplug_dev, &lpc->pm.acpi_memory_hotplug,
+dev, errp);
+}
} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
if (lpc->pm.cpu_hotplug_legacy) {
legacy_acpi_cpu_plug_cb(hotplug_dev, &lpc->pm.gpe_cpu, dev, errp);
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 593ac0d..b8548cc 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -874,6 +874,13 @@ static const MemoryRegionOps nvdimm_dsm_ops = {
},
};
+void nvdimm_acpi_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev)
+{
+if (dev->hotplugged) {
+acpi_send_event(DEVICE(hotplug_dev), ACPI_NVDIMM_HOTPLUG_STATUS);
+}
+}
+
void nvdimm_init_acpi_state(AcpiNVDIMMState *state, MemoryRegion *io,
FWCfgState *fw_cfg, Object *owner)
{
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 2adc246..17d36bd 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -378,7 +378,12 @@ static void piix4_device_plug_cb(HotplugHandler
*hotplug_dev,
if (s->acpi_memory_hotplug.is_enabled &&
object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
-acpi_memory_plug_cb(hotplug_dev, &s->acpi_memory_hotplug, dev, errp);
+if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
+nvdimm_acpi_plug_cb(hotplug_dev, dev);
+} else {
+acpi_memory_plug_cb(hotplug_dev, &s->acpi_memory_hotplug,
+dev, errp);
+}
} else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
acpi_pcihp_device_plug_cb(hotplug_dev, &s->acpi_pci_hotplug, dev,
errp);
} else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index bc49958..32270c3 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2039,6 +2039,13 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
method = aml_method("_E03", 0, AML_NOTSERIALIZED);
aml_append(method, aml_call0(MEMORY_HOTPLUG_HANDLER_PATH));
aml_append(scope, method);
+
+if (pcms->acpi_nvdimm_state.is_enabled) {
+method = aml_method("_E04", 0, AML_NOTSERIALIZED);
+aml_append(method, aml_notify(aml_name("\\_SB.NVDR"),
+ aml_int(0x80)));
+aml_append(scope, method);
+}
}
aml_append(dsdt, scope);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 77ca7f4..0403452 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1723,6 +1723,12 @@ static void pc_dimm_unplug_request(HotplugHandler
*hotplug_dev,
goto out;
}
+if (object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM)) {
+error_setg(&local_err,
+ "n
[Qemu-devel] [PATCH v4 1/3] nvdimm acpi: introduce fit buffer
The buffer is used to save the FIT info for all the presented nvdimm
devices which is updated after the nvdimm device is plugged or
unplugged. In the later patch, it will be used to construct NVDIMM
ACPI _FIT method which reflects the presented nvdimm devices after
nvdimm hotplug
As FIT buffer can not completely mapped into guest address space,
OSPM will exit to QEMU multiple times, however, there is the race
condition - FIT may be changed during these multiple exits, so that
we mark @dirty whenever the buffer is updated.
@dirty is cleared for the first time OSPM gets fit buffer, if
dirty is detected in the later access, OSPM will restart the
access
Signed-off-by: Xiao Guangrong
---
hw/acpi/nvdimm.c| 57 ++---
hw/i386/acpi-build.c| 2 +-
hw/i386/pc.c| 4
include/hw/mem/nvdimm.h | 21 +-
4 files changed, 60 insertions(+), 24 deletions(-)
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index b8a2e62..9fee077 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -38,11 +38,7 @@ static int nvdimm_plugged_device_list(Object *obj, void
*opaque)
GSList **list = opaque;
if (object_dynamic_cast(obj, TYPE_NVDIMM)) {
-DeviceState *dev = DEVICE(obj);
-
-if (dev->realized) { /* only realized NVDIMMs matter */
-*list = g_slist_append(*list, DEVICE(obj));
-}
+*list = g_slist_append(*list, DEVICE(obj));
}
object_child_foreach(obj, nvdimm_plugged_device_list, opaque);
@@ -348,8 +344,9 @@ static void nvdimm_build_structure_dcr(GArray *structures,
DeviceState *dev)
(DSM) in DSM Spec Rev1.*/);
}
-static GArray *nvdimm_build_device_structure(GSList *device_list)
+static GArray *nvdimm_build_device_structure(void)
{
+GSList *device_list = nvdimm_get_plugged_device_list();
GArray *structures = g_array_new(false, true /* clear */, 1);
for (; device_list; device_list = device_list->next) {
@@ -367,28 +364,50 @@ static GArray *nvdimm_build_device_structure(GSList
*device_list)
/* build NVDIMM Control Region Structure. */
nvdimm_build_structure_dcr(structures, dev);
}
+g_slist_free(device_list);
return structures;
}
-static void nvdimm_build_nfit(GSList *device_list, GArray *table_offsets,
+static void nvdimm_init_fit_buffer(NvdimmFitBuffer *fit_buf)
+{
+fit_buf->fit = g_array_new(false, true /* clear */, 1);
+}
+
+static void nvdimm_build_fit_buffer(NvdimmFitBuffer *fit_buf)
+{
+g_array_free(fit_buf->fit, true);
+fit_buf->fit = nvdimm_build_device_structure();
+fit_buf->dirty = true;
+}
+
+void nvdimm_acpi_hotplug(AcpiNVDIMMState *state)
+{
+nvdimm_build_fit_buffer(&state->fit_buf);
+}
+
+static void nvdimm_build_nfit(AcpiNVDIMMState *state, GArray *table_offsets,
GArray *table_data, BIOSLinker *linker)
{
-GArray *structures = nvdimm_build_device_structure(device_list);
+NvdimmFitBuffer *fit_buf = &state->fit_buf;
unsigned int header;
+/* NVDIMM device is not plugged? */
+if (!fit_buf->fit->len) {
+return;
+}
+
acpi_add_table(table_offsets, table_data);
/* NFIT header. */
header = table_data->len;
acpi_data_push(table_data, sizeof(NvdimmNfitHeader));
/* NVDIMM device structures. */
-g_array_append_vals(table_data, structures->data, structures->len);
+g_array_append_vals(table_data, fit_buf->fit->data, fit_buf->fit->len);
build_header(linker, table_data,
(void *)(table_data->data + header), "NFIT",
- sizeof(NvdimmNfitHeader) + structures->len, 1, NULL, NULL);
-g_array_free(structures, true);
+ sizeof(NvdimmNfitHeader) + fit_buf->fit->len, 1, NULL, NULL);
}
struct NvdimmDsmIn {
@@ -771,6 +790,8 @@ void nvdimm_init_acpi_state(AcpiNVDIMMState *state,
MemoryRegion *io,
acpi_data_push(state->dsm_mem, sizeof(NvdimmDsmIn));
fw_cfg_add_file(fw_cfg, NVDIMM_DSM_MEM_FILE, state->dsm_mem->data,
state->dsm_mem->len);
+
+nvdimm_init_fit_buffer(&state->fit_buf);
}
#define NVDIMM_COMMON_DSM "NCAL"
@@ -1045,25 +1066,17 @@ static void nvdimm_build_ssdt(GArray *table_offsets,
GArray *table_data,
}
void nvdimm_build_acpi(GArray *table_offsets, GArray *table_data,
- BIOSLinker *linker, GArray *dsm_dma_arrea,
+ BIOSLinker *linker, AcpiNVDIMMState *state,
uint32_t ram_slots)
{
-GSList *device_list;
-
-device_list = nvdimm_get_plugged_device_list();
-
-/* NVDIMM device is plugged. */
-if (device_list) {
-nvdimm_build_nfit(device_list, table_offsets, table_data, linker);
-g_slist_free(device_list);
-}
+nvdimm_build_nfit(state, table_offsets, table_data, linker);
/*
* NVDIMM device is allowed to be plugged only if there is available
*
[Qemu-devel] [PATCH v4 0/3] nvdimm: hotplug support
Resend these 3 patches to catch up release window... Igor, this is a open that i did not pass a buffer as parameter to RFIT as tried the way you suggested, but failed. May be i am not very good at ASL, i need more time to try. So let's keep the way as it is, i will improve it later. Thanks! Changelog in v4: 1) drop fit lock and post_hotplug_cb 2) move nvdimm hotplug code to hw/acpi/nvdimm.c 3) introduce length field to indicate the fit size 4) nvdimm acpi cleanup 5) doc typo fixes Changelog in v3: 1) use a dedicated interrupt for nvdimm device hotplug 2) stop nvdimm device hot unplug 3) reserve UUID and handle for QEMU internally used QEMU 5) redesign fit buffer to avoid OSPM reading incomplete fit info 6) bug fixes and cleanups Changelog in v2: Fixed signed integer overflow pointed out by Stefan Hajnoczi This patchset enables nvdimm hotplug support, it is used as pc-dimm hotplug, for example, a new nvdimm device can be plugged as follows: object_add memory-backend-file,id=mem3,size=10G,mem-path=/home/eric/nvdimm3 device_add nvdimm,id=nvdimm3,memdev=mem3 and unplug it as follows: device_del nvdimm3 Xiao Guangrong (3): nvdimm acpi: introduce fit buffer nvdimm acpi: introduce _FIT pc: memhp: enable nvdimm device hotplug default-configs/mips-softmmu-common.mak | 1 + docs/specs/acpi_nvdimm.txt | 68 +++- hw/acpi/ich9.c | 8 +- hw/acpi/nvdimm.c| 289 hw/acpi/piix4.c | 7 +- hw/i386/acpi-build.c| 9 +- hw/i386/pc.c| 16 ++ hw/mem/nvdimm.c | 4 - include/hw/acpi/acpi_dev_interface.h| 1 + include/hw/mem/nvdimm.h | 22 ++- 10 files changed, 377 insertions(+), 48 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH v5 03/10] pci: Convert msix_init() to Error and fix callers to check it
msix_init() reports errors with error_report(), which is wrong when
it's used in realize(). The same issue was fixed for msi_init() in
commit 1108b2f.
For some devices(like e1000e, vmxnet3) who won't fail because of
msix_init's failure, suppress the error report by passing NULL error object.
Bonus: add comment for msix_init.
CC: Jiri Pirko
CC: Gerd Hoffmann
CC: Dmitry Fleytman
CC: Jason Wang
CC: Michael S. Tsirkin
CC: Hannes Reinecke
CC: Paolo Bonzini
CC: Alex Williamson
CC: Markus Armbruster
CC: Marcel Apfelbaum
Reviewed-by: Markus Armbruster
Signed-off-by: Cao jin
---
hw/block/nvme.c| 5 -
hw/misc/ivshmem.c | 8
hw/net/e1000e.c| 6 +-
hw/net/rocker/rocker.c | 7 ++-
hw/net/vmxnet3.c | 8 ++--
hw/pci/msix.c | 34 +-
hw/scsi/megasas.c | 5 -
hw/usb/hcd-xhci.c | 13 -
hw/vfio/pci.c | 4 +++-
hw/virtio/virtio-pci.c | 11 +--
include/hw/pci/msix.h | 5 +++--
11 files changed, 77 insertions(+), 29 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index d479fd2..2d703c8 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -831,6 +831,7 @@ static int nvme_init(PCIDevice *pci_dev)
{
NvmeCtrl *n = NVME(pci_dev);
NvmeIdCtrl *id = &n->id_ctrl;
+Error *err = NULL;
int i;
int64_t bs_size;
@@ -872,7 +873,9 @@ static int nvme_init(PCIDevice *pci_dev)
pci_register_bar(&n->parent_obj, 0,
PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
&n->iomem);
-msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4);
+if (msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, &err)) {
+error_report_err(err);
+}
id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 230e51b..ca6f804 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -749,13 +749,13 @@ static void ivshmem_reset(DeviceState *d)
}
}
-static int ivshmem_setup_interrupts(IVShmemState *s)
+static int ivshmem_setup_interrupts(IVShmemState *s, Error **errp)
{
/* allocate QEMU callback data for receiving interrupts */
s->msi_vectors = g_malloc0(s->vectors * sizeof(MSIVector));
if (ivshmem_has_feature(s, IVSHMEM_MSI)) {
-if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1)) {
+if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1, errp)) {
return -1;
}
@@ -897,8 +897,8 @@ static void ivshmem_common_realize(PCIDevice *dev, Error
**errp)
qemu_chr_fe_set_handlers(&s->server_chr, ivshmem_can_receive,
ivshmem_read, NULL, s, NULL, true);
-if (ivshmem_setup_interrupts(s) < 0) {
-error_setg(errp, "failed to initialize interrupts");
+if (ivshmem_setup_interrupts(s, errp) < 0) {
+error_prepend(errp, "Failed to initialize interrupts: ");
return;
}
}
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 4994e1c..74cbbef 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -292,7 +292,11 @@ e1000e_init_msix(E1000EState *s)
E1000E_MSIX_IDX, E1000E_MSIX_TABLE,
&s->msix,
E1000E_MSIX_IDX, E1000E_MSIX_PBA,
-0xA0);
+0xA0, NULL);
+
+/* Any error other than -ENOTSUP(board's MSI support is broken)
+ * is a programming error. Fall back to INTx silently on -ENOTSUP */
+assert(!res || res == -ENOTSUP);
if (res < 0) {
trace_e1000e_msix_init_fail(res);
diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index e9d215a..8f829f2 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -1256,14 +1256,19 @@ static int rocker_msix_init(Rocker *r)
{
PCIDevice *dev = PCI_DEVICE(r);
int err;
+Error *local_err = NULL;
err = msix_init(dev, ROCKER_MSIX_VEC_COUNT(r->fp_ports),
&r->msix_bar,
ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_TABLE_OFFSET,
&r->msix_bar,
ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_PBA_OFFSET,
-0);
+0, &local_err);
+/* Any error other than -ENOTSUP(board's MSI support is broken)
+ * is a programming error. */
+assert(!err || err == -ENOTSUP);
if (err) {
+error_report_err(local_err);
return err;
}
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 92f6af9..a433cc0 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2191,10 +2191,14 @@ vmxnet3_init_msix(VMXNET3State *s)
VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_TABLE,
&s->msix_bar,
VMXNET3_MSIX_BAR_IDX, VMXNET3_OFF_MSIX_PBA(s
[Qemu-devel] [PATCH v5 02/10] hcd-xhci: check & correct param before using it
usb_xhci_realize() corrects invalid values of property "intrs"
automatically, but the uncorrected value is passed to msi_init(),
which chokes on invalid values. Delay that until after the
correction.
Resources allocated by usb_xhci_init() are leaked when msi_init()
fails. Fix by calling it after msi_init().
CC: Gerd Hoffmann
CC: Markus Armbruster
CC: Marcel Apfelbaum
CC: Michael S. Tsirkin
Reviewed-by: Gerd Hoffmann
Reviewed-by: Markus Armbruster
Signed-off-by: Cao jin
---
hw/usb/hcd-xhci.c | 39 +++
1 file changed, 19 insertions(+), 20 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 4acf0c6..eb1dca5 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3627,25 +3627,6 @@ static void usb_xhci_realize(struct PCIDevice *dev,
Error **errp)
dev->config[PCI_CACHE_LINE_SIZE] = 0x10;
dev->config[0x60] = 0x30; /* release number */
-usb_xhci_init(xhci);
-
-if (xhci->msi != ON_OFF_AUTO_OFF) {
-ret = msi_init(dev, 0x70, xhci->numintrs, true, false, &err);
-/* Any error other than -ENOTSUP(board's MSI support is broken)
- * is a programming error */
-assert(!ret || ret == -ENOTSUP);
-if (ret && xhci->msi == ON_OFF_AUTO_ON) {
-/* Can't satisfy user's explicit msi=on request, fail */
-error_append_hint(&err, "You have to use msi=auto (default) or "
-"msi=off with this machine type.\n");
-error_propagate(errp, err);
-return;
-}
-assert(!err || xhci->msi == ON_OFF_AUTO_AUTO);
-/* With msi=auto, we fall back to MSI off silently */
-error_free(err);
-}
-
if (xhci->numintrs > MAXINTRS) {
xhci->numintrs = MAXINTRS;
}
@@ -3667,7 +3648,22 @@ static void usb_xhci_realize(struct PCIDevice *dev,
Error **errp)
xhci->max_pstreams_mask = 0;
}
-xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_mfwrap_timer,
xhci);
+if (xhci->msi != ON_OFF_AUTO_OFF) {
+ret = msi_init(dev, 0x70, xhci->numintrs, true, false, &err);
+/* Any error other than -ENOTSUP(board's MSI support is broken)
+ * is a programming error */
+assert(!ret || ret == -ENOTSUP);
+if (ret && xhci->msi == ON_OFF_AUTO_ON) {
+/* Can't satisfy user's explicit msi=on request, fail */
+error_append_hint(&err, "You have to use msi=auto (default) or "
+"msi=off with this machine type.\n");
+error_propagate(errp, err);
+return;
+}
+assert(!err || xhci->msi == ON_OFF_AUTO_AUTO);
+/* With msi=auto, we fall back to MSI off silently */
+error_free(err);
+}
memory_region_init(&xhci->mem, OBJECT(xhci), "xhci", LEN_REGS);
memory_region_init_io(&xhci->mem_cap, OBJECT(xhci), &xhci_cap_ops, xhci,
@@ -3697,6 +3693,9 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error
**errp)
PCI_BASE_ADDRESS_SPACE_MEMORY|PCI_BASE_ADDRESS_MEM_TYPE_64,
&xhci->mem);
+usb_xhci_init(xhci);
+xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_mfwrap_timer,
xhci);
+
if (pci_bus_is_express(dev->bus) ||
xhci_get_flag(xhci, XHCI_FLAG_FORCE_PCIE_ENDCAP)) {
ret = pcie_endpoint_cap_init(dev, 0xa0);
--
2.1.0
[Qemu-devel] [PATCH v4 2/3] nvdimm acpi: introduce _FIT
_FIT is required for hotplug support, guest will inquire the updated
device info from it if a hotplug event is received
As FIT buffer is not completely mapped into guest address space, so a
new function, Read FIT whose UUID is UUID
648B9CF2-CDA1-4312-8AD9-49C4AF32BD62, handle 0x1, function index
is 0x1, is reserved by QEMU to read the piece of FIT buffer. The buffer
is concatenated before _FIT return
Refer to docs/specs/acpi-nvdimm.txt for detailed design
Signed-off-by: Xiao Guangrong
---
docs/specs/acpi_nvdimm.txt | 63 -
hw/acpi/nvdimm.c | 225 ++---
2 files changed, 271 insertions(+), 17 deletions(-)
diff --git a/docs/specs/acpi_nvdimm.txt b/docs/specs/acpi_nvdimm.txt
index 0fdd251..364e832 100644
--- a/docs/specs/acpi_nvdimm.txt
+++ b/docs/specs/acpi_nvdimm.txt
@@ -65,8 +65,8 @@ _FIT(Firmware Interface Table)
The detailed definition of the structure can be found at ACPI 6.0: 5.2.25
NVDIMM Firmware Interface Table (NFIT).
-QEMU NVDIMM Implemention
-
+QEMU NVDIMM Implementation
+==
QEMU uses 4 bytes IO Port starting from 0x0a18 and a RAM-based memory page
for NVDIMM ACPI.
@@ -82,6 +82,16 @@ Memory:
ACPI writes _DSM Input Data (based on the offset in the page):
[0x0 - 0x3]: 4 bytes, NVDIMM Device Handle, 0 is reserved for NVDIMM
Root device.
+
+The handle is completely QEMU internal thing, the values in
+range [0, 0x] indicate nvdimm device (O means nvdimm
+root device named NVDR), other values are reserved by other
+purpose.
+
+Current reserved handle:
+0x1 is reserved for QEMU internal DSM function called on
+the root device.
+
[0x4 - 0x7]: 4 bytes, Revision ID, that is the Arg1 of _DSM method.
[0x8 - 0xB]: 4 bytes. Function Index, that is the Arg2 of _DSM method.
[0xC - 0xFFF]: 4084 bytes, the Arg3 of _DSM method.
@@ -127,6 +137,49 @@ _DSM process diagram:
| result from the page | | |
+--+ +--+
- _FIT implementation
- ---
- TODO (will fill it when nvdimm hotplug is introduced)
+QEMU internal use only _DSM function
+
+There is the function introduced by QEMU and only used by QEMU internal.
+
+1) Read FIT
+ UUID 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62 is reserved for Read_FIT DSM
+ function (private QEMU function)
+
+ _FIT method uses Read_FIT function to fetch NFIT structures blob from
+ QEMU in 1 page sized increments which are then concatenated and returned
+ as _FIT method result.
+
+ Input parameters:
+ Arg0 – UUID {set to 648B9CF2-CDA1-4312-8AD9-49C4AF32BD62}
+ Arg1 – Revision ID (set to 1)
+ Arg2 - Function Index, 0x1
+ Arg3 - A package containing a buffer whose layout is as follows:
+
+ +--+++---+
+ | Field | Length | Offset | Description |
+ +--+++---+
+ | offset | 4| 0| offset in QEMU's NFIT structures blob to |
+ | ||| read from |
+ +--+++---+
+
+ Output:
+ +--+++---+
+ | Field | Length | Offset | Description |
+ +--+++---+
+ | ||| return status codes |
+ | ||| 0x100 - error caused by NFIT update while |
+ | status | 4| 0| read by _FIT wasn't completed, other |
+ | ||| codes follow Chapter 3 in DSM Spec Rev1 |
+ +--+++---+
+ | length | 4| 4| The fit size |
+ +--+-+---+
+ | fit data | Varies | 8| FIT data, its size is indicated by length |
+ | ||| filed above |
+ +--+++---+
+
+ The FIT offset is maintained by the OSPM itself, current offset plus
+ the length returned by the function is the next offset we should read.
+ When all the FIT data has been read out, zero length is returned.
+
+ If it returns 0x100, OSPM should restart to read FIT (read from offset 0
+ again).
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 9fee077..593ac0d 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -484,6 +484,23 @@ typedef struct NvdimmFuncSetLabelDataIn
Nvdi
[Qemu-devel] [PATCH v5 09/10] vmxnet3: remove unnecessary internal msix flag
Internal flag msix_used is unnecessary, it has the same effect as
msix_enabled().
The corresponding msi flag is already dropped in commit 1070048e.
CC: Dmitry Fleytman
CC: Jason Wang
CC: Markus Armbruster
CC: Michael S. Tsirkin
Reviewed-by: Markus Armbruster
Reviewed-by: Dmitry Fleytman
Signed-off-by: Cao jin
---
hw/net/vmxnet3.c | 28 ++--
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 45e125e..af39965 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -281,8 +281,6 @@ typedef struct {
Vmxnet3RxqDescr rxq_descr[VMXNET3_DEVICE_MAX_RX_QUEUES];
Vmxnet3TxqDescr txq_descr[VMXNET3_DEVICE_MAX_TX_QUEUES];
-/* Whether MSI-X support was installed successfully */
-bool msix_used;
hwaddr drv_shmem;
hwaddr temp_shared_guest_driver_memory;
@@ -359,7 +357,7 @@ static bool _vmxnet3_assert_interrupt_line(VMXNET3State *s,
uint32_t int_idx)
{
PCIDevice *d = PCI_DEVICE(s);
-if (s->msix_used && msix_enabled(d)) {
+if (msix_enabled(d)) {
VMW_IRPRN("Sending MSI-X notification for vector %u", int_idx);
msix_notify(d, int_idx);
return false;
@@ -383,7 +381,7 @@ static void _vmxnet3_deassert_interrupt_line(VMXNET3State
*s, int lidx)
* This function should never be called for MSI(X) interrupts
* because deassertion never required for message interrupts
*/
-assert(!s->msix_used || !msix_enabled(d));
+assert(!msix_enabled(d));
/*
* This function should never be called for MSI(X) interrupts
* because deassertion never required for message interrupts
@@ -421,7 +419,7 @@ static void vmxnet3_trigger_interrupt(VMXNET3State *s, int
lidx)
s->interrupt_states[lidx].is_pending = true;
vmxnet3_update_interrupt_line_state(s, lidx);
-if (s->msix_used && msix_enabled(d) && s->auto_int_masking) {
+if (msix_enabled(d) && s->auto_int_masking) {
goto do_automask;
}
@@ -1428,7 +1426,7 @@ static void vmxnet3_update_features(VMXNET3State *s)
static bool vmxnet3_verify_intx(VMXNET3State *s, int intx)
{
-return s->msix_used || msi_enabled(PCI_DEVICE(s))
+return msix_enabled(PCI_DEVICE(s)) || msi_enabled(PCI_DEVICE(s))
|| intx == pci_get_byte(s->parent_obj.config + PCI_INTERRUPT_PIN) - 1;
}
@@ -1445,18 +1443,18 @@ static void vmxnet3_validate_interrupts(VMXNET3State *s)
int i;
VMW_CFPRN("Verifying event interrupt index (%d)", s->event_int_idx);
-vmxnet3_validate_interrupt_idx(s->msix_used, s->event_int_idx);
+vmxnet3_validate_interrupt_idx(msix_enabled(PCI_DEVICE(s)),
s->event_int_idx);
for (i = 0; i < s->txq_num; i++) {
int idx = s->txq_descr[i].intr_idx;
VMW_CFPRN("Verifying TX queue %d interrupt index (%d)", i, idx);
-vmxnet3_validate_interrupt_idx(s->msix_used, idx);
+vmxnet3_validate_interrupt_idx(msix_enabled(PCI_DEVICE(s)), idx);
}
for (i = 0; i < s->rxq_num; i++) {
int idx = s->rxq_descr[i].intr_idx;
VMW_CFPRN("Verifying RX queue %d interrupt index (%d)", i, idx);
-vmxnet3_validate_interrupt_idx(s->msix_used, idx);
+vmxnet3_validate_interrupt_idx(msix_enabled(PCI_DEVICE(s)), idx);
}
}
@@ -2185,6 +2183,7 @@ vmxnet3_use_msix_vectors(VMXNET3State *s, int num_vectors)
static bool
vmxnet3_init_msix(VMXNET3State *s)
{
+bool msix;
PCIDevice *d = PCI_DEVICE(s);
int res = msix_init(d, VMXNET3_MAX_INTRS,
&s->msix_bar,
@@ -2199,17 +2198,18 @@ vmxnet3_init_msix(VMXNET3State *s)
if (0 > res) {
VMW_WRPRN("Failed to initialize MSI-X, board's MSI support is broken");
-s->msix_used = false;
+msix = false;
} else {
if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS)) {
VMW_WRPRN("Failed to use MSI-X vectors, error %d", res);
msix_uninit(d, &s->msix_bar, &s->msix_bar);
-s->msix_used = false;
+msix = false;
} else {
-s->msix_used = true;
+msix = true;
}
}
-return s->msix_used;
+
+return msix;
}
static void
@@ -2217,7 +2217,7 @@ vmxnet3_cleanup_msix(VMXNET3State *s)
{
PCIDevice *d = PCI_DEVICE(s);
-if (s->msix_used) {
+if (msix_enabled(d)) {
vmxnet3_unuse_msix_vectors(s, VMXNET3_MAX_INTRS);
msix_uninit(d, &s->msix_bar, &s->msix_bar);
}
--
2.1.0
[Qemu-devel] [PATCH v5 06/10] megasas: remove unnecessary megasas_use_msix()
Also move certain hunk above, to place msix init related code together.
CC: Hannes Reinecke
CC: Paolo Bonzini
CC: Markus Armbruster
CC: Marcel Apfelbaum
CC: Michael S. Tsirkin
Reviewed-by: Markus Armbruster
Signed-off-by: Cao jin
---
hw/scsi/megasas.c | 19 ++-
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 6cef9a3..ba79e7a 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -155,11 +155,6 @@ static bool megasas_use_queue64(MegasasState *s)
return s->flags & MEGASAS_MASK_USE_QUEUE64;
}
-static bool megasas_use_msix(MegasasState *s)
-{
-return s->msix != ON_OFF_AUTO_OFF;
-}
-
static bool megasas_is_jbod(MegasasState *s)
{
return s->flags & MEGASAS_MASK_USE_JBOD;
@@ -2299,9 +2294,7 @@ static void megasas_scsi_uninit(PCIDevice *d)
{
MegasasState *s = MEGASAS(d);
-if (megasas_use_msix(s)) {
-msix_uninit(d, &s->mmio_io, &s->mmio_io);
-}
+msix_uninit(d, &s->mmio_io, &s->mmio_io);
msi_uninit(d);
}
@@ -2353,7 +2346,7 @@ static void megasas_scsi_realize(PCIDevice *dev, Error
**errp)
memory_region_init_io(&s->mmio_io, OBJECT(s), &megasas_mmio_ops, s,
"megasas-mmio", 0x4000);
-if (megasas_use_msix(s)) {
+if (s->msix != ON_OFF_AUTO_OFF) {
ret = msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
&s->mmio_io, b->mmio_bar, 0x3800, 0x68, &err);
/* Any error other than -ENOTSUP(board's MSI support is broken)
@@ -2373,6 +2366,10 @@ static void megasas_scsi_realize(PCIDevice *dev, Error
**errp)
error_free(err);
}
+if (msix_enabled(dev)) {
+msix_vector_use(dev, 0);
+}
+
memory_region_init_io(&s->port_io, OBJECT(s), &megasas_port_ops, s,
"megasas-io", 256);
memory_region_init_io(&s->queue_io, OBJECT(s), &megasas_queue_ops, s,
@@ -2388,10 +2385,6 @@ static void megasas_scsi_realize(PCIDevice *dev, Error
**errp)
pci_register_bar(dev, b->mmio_bar, bar_type, &s->mmio_io);
pci_register_bar(dev, 3, bar_type, &s->queue_io);
-if (megasas_use_msix(s)) {
-msix_vector_use(dev, 0);
-}
-
s->fw_state = MFI_FWSTATE_READY;
if (!s->sas_addr) {
s->sas_addr = ((NAA_LOCALLY_ASSIGNED_ID << 24) |
--
2.1.0
[Qemu-devel] [PATCH v5 10/10] msi_init: convert assert to return -errno
According to the disscussion:
http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg08215.html
Let leaf function returns reasonable -errno, let caller decide how to
handle the return value.
Suggested-by: Markus Armbruster
CC: Markus Armbruster
CC: Michael S. Tsirkin
CC: Marcel Apfelbaum
Reviewed-by: Markus Armbruster
Signed-off-by: Cao jin
---
hw/pci/msi.c | 11 ---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a87b227..443682b 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -201,9 +201,14 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
" 64bit %d mask %d\n",
offset, nr_vectors, msi64bit, msi_per_vector_mask);
-assert(!(nr_vectors & (nr_vectors - 1))); /* power of 2 */
-assert(nr_vectors > 0);
-assert(nr_vectors <= PCI_MSI_VECTORS_MAX);
+/* vector sanity test: should in range 1 - 32, should be power of 2 */
+if ((nr_vectors == 0) ||
+(nr_vectors > PCI_MSI_VECTORS_MAX) ||
+(nr_vectors & (nr_vectors - 1))) {
+error_setg(errp, "Invalid vector number: %d", nr_vectors);
+return -EINVAL;
+}
+
/* the nr of MSI vectors is up to 32 */
vectors_order = ctz32(nr_vectors);
--
2.1.0
[Qemu-devel] [PATCH v5 04/10] megasas: change behaviour of msix switch
Resolve the TODO, msix=auto means msix on; if user specify msix=on,
then device creation fail on msix_init failure.
Also undo the overwrites of user configuration of msix.
CC: Michael S. Tsirkin
CC: Hannes Reinecke
CC: Paolo Bonzini
CC: Markus Armbruster
CC: Marcel Apfelbaum
Reviewed-by: Markus Armbruster
Signed-off-by: Cao jin
---
hw/scsi/megasas.c | 28
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index fada834..6cef9a3 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2353,19 +2353,31 @@ static void megasas_scsi_realize(PCIDevice *dev, Error
**errp)
memory_region_init_io(&s->mmio_io, OBJECT(s), &megasas_mmio_ops, s,
"megasas-mmio", 0x4000);
+if (megasas_use_msix(s)) {
+ret = msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
+&s->mmio_io, b->mmio_bar, 0x3800, 0x68, &err);
+/* Any error other than -ENOTSUP(board's MSI support is broken)
+ * is a programming error */
+assert(!ret || ret == -ENOTSUP);
+if (ret && s->msix == ON_OFF_AUTO_ON) {
+/* Can't satisfy user's explicit msix=on request, fail */
+error_append_hint(&err, "You have to use msix=auto (default) or "
+"msix=off with this machine type.\n");
+/* No instance_finalize method, need to free the resource here */
+object_unref(OBJECT(&s->mmio_io));
+error_propagate(errp, err);
+return;
+}
+assert(!err || s->msix == ON_OFF_AUTO_AUTO);
+/* With msix=auto, we fall back to MSI off silently */
+error_free(err);
+}
+
memory_region_init_io(&s->port_io, OBJECT(s), &megasas_port_ops, s,
"megasas-io", 256);
memory_region_init_io(&s->queue_io, OBJECT(s), &megasas_queue_ops, s,
"megasas-queue", 0x4);
-if (megasas_use_msix(s) &&
-msix_init(dev, 15, &s->mmio_io, b->mmio_bar, 0x2000,
- &s->mmio_io, b->mmio_bar, 0x3800, 0x68, &err)) {
-/*TODO: check msix_init's error, and should fail on msix=on */
-error_report_err(err);
-s->msix = ON_OFF_AUTO_OFF;
-}
-
if (pci_is_express(dev)) {
pcie_endpoint_cap_init(dev, 0xa0);
}
--
2.1.0
[Qemu-devel] [PATCH v5 08/10] vmxnet3: fix reference leak issue
On migration target, msix_vector_use() will be called in vmxnet3_post_load()
in second time, without a matching second call to msi_vector_unuse(),
which results in vector reference leak.
CC: Dmitry Fleytman
CC: Jason Wang
CC: Markus Armbruster
CC: Michael S. Tsirkin
Reviewed-by: Markus Armbruster
Reviewed-by: Dmitry Fleytman
Signed-off-by: Cao jin
---
hw/net/vmxnet3.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index a433cc0..45e125e 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -2552,21 +2552,11 @@ static void vmxnet3_put_rxq_descr(QEMUFile *f, void
*pv, size_t size)
static int vmxnet3_post_load(void *opaque, int version_id)
{
VMXNET3State *s = opaque;
-PCIDevice *d = PCI_DEVICE(s);
net_tx_pkt_init(&s->tx_pkt, PCI_DEVICE(s),
s->max_tx_frags, s->peer_has_vhdr);
net_rx_pkt_init(&s->rx_pkt, s->peer_has_vhdr);
-if (s->msix_used) {
-if (!vmxnet3_use_msix_vectors(s, VMXNET3_MAX_INTRS)) {
-VMW_WRPRN("Failed to re-use MSI-X vectors");
-msix_uninit(d, &s->msix_bar, &s->msix_bar);
-s->msix_used = false;
-return -1;
-}
-}
-
vmxnet3_validate_queues(s);
vmxnet3_validate_interrupts(s);
--
2.1.0
[Qemu-devel] [PATCH v5 05/10] hcd-xhci: change behaviour of msix switch
Resolve the TODO, msix=auto means msix on; if user specify msix=on,
then device creation fail on msix_init failure.
CC: Gerd Hoffmann
CC: Michael S. Tsirkin
CC: Markus Armbruster
CC: Marcel Apfelbaum
Reviewed-by: Gerd Hoffmann
Reviewed-by: Markus Armbruster
Signed-off-by: Cao jin
---
hw/usb/hcd-xhci.c | 35 ---
1 file changed, 24 insertions(+), 11 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 05dc944..4767045 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3636,12 +3636,14 @@ static void usb_xhci_realize(struct PCIDevice *dev,
Error **errp)
if (xhci->numintrs < 1) {
xhci->numintrs = 1;
}
+
if (xhci->numslots > MAXSLOTS) {
xhci->numslots = MAXSLOTS;
}
if (xhci->numslots < 1) {
xhci->numslots = 1;
}
+
if (xhci_get_flag(xhci, XHCI_FLAG_ENABLE_STREAMS)) {
xhci->max_pstreams_mask = 7; /* == 256 primary streams */
} else {
@@ -3666,6 +3668,28 @@ static void usb_xhci_realize(struct PCIDevice *dev,
Error **errp)
}
memory_region_init(&xhci->mem, OBJECT(xhci), "xhci", LEN_REGS);
+if (xhci->msix != ON_OFF_AUTO_OFF) {
+ret = msix_init(dev, xhci->numintrs,
+&xhci->mem, 0, OFF_MSIX_TABLE,
+&xhci->mem, 0, OFF_MSIX_PBA,
+0x90, &err);
+/* Any error other than -ENOTSUP(board's MSI support is broken)
+ * is a programming error */
+assert(!ret || ret == -ENOTSUP);
+if (ret && xhci->msix == ON_OFF_AUTO_ON) {
+/* Can't satisfy user's explicit msix=on request, fail */
+error_append_hint(&err, "You have to use msix=auto (default) or "
+"msix=off with this machine type.\n");
+/* No instance_finalize method, need to free the resource here */
+object_unref(OBJECT(&xhci->mem));
+error_propagate(errp, err);
+return;
+}
+assert(!err || xhci->msix == ON_OFF_AUTO_AUTO);
+/* With msix=auto, we fall back to MSI off silently */
+error_free(err);
+}
+
memory_region_init_io(&xhci->mem_cap, OBJECT(xhci), &xhci_cap_ops, xhci,
"capabilities", LEN_CAP);
memory_region_init_io(&xhci->mem_oper, OBJECT(xhci), &xhci_oper_ops, xhci,
@@ -3701,17 +3725,6 @@ static void usb_xhci_realize(struct PCIDevice *dev,
Error **errp)
ret = pcie_endpoint_cap_init(dev, 0xa0);
assert(ret >= 0);
}
-
-if (xhci->msix != ON_OFF_AUTO_OFF) {
-/* TODO check for errors, and should fail when msix=on */
-ret = msix_init(dev, xhci->numintrs,
-&xhci->mem, 0, OFF_MSIX_TABLE,
-&xhci->mem, 0, OFF_MSIX_PBA,
-0x90, &err);
-if (ret) {
-error_report_err(err);
-}
-}
}
static void usb_xhci_exit(PCIDevice *dev)
--
2.1.0
[Qemu-devel] [PATCH v5 01/10] msix: Follow CODING_STYLE
CC: Markus Armbruster
CC: Marcel Apfelbaum
CC: Michael S. Tsirkin
Reviewed-by: Markus Armbruster
Signed-off-by: Cao jin
---
hw/pci/msix.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 0ec1cb1..0cee631 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -447,8 +447,10 @@ void msix_notify(PCIDevice *dev, unsigned vector)
{
MSIMessage msg;
-if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector])
+if (vector >= dev->msix_entries_nr || !dev->msix_entry_used[vector]) {
return;
+}
+
if (msix_is_masked(dev, vector)) {
msix_set_pending(dev, vector);
return;
@@ -483,8 +485,10 @@ void msix_reset(PCIDevice *dev)
/* Mark vector as used. */
int msix_vector_use(PCIDevice *dev, unsigned vector)
{
-if (vector >= dev->msix_entries_nr)
+if (vector >= dev->msix_entries_nr) {
return -EINVAL;
+}
+
dev->msix_entry_used[vector]++;
return 0;
}
--
2.1.0
[Qemu-devel] [PATCH v5 00/10] Convert msix_init() to error
v5 changelog: 1. Address to all comments of Markus 2. Add the given R-b to each commit message. Cao jin (10): msix: Follow CODING_STYLE hcd-xhci: check & correct param before using it pci: Convert msix_init() to Error and fix callers to check it megasas: change behaviour of msix switch hcd-xhci: change behaviour of msix switch megasas: remove unnecessary megasas_use_msix() megasas: undo the overwrites of msi user configuration vmxnet3: fix reference leak issue vmxnet3: remove unnecessary internal msix flag msi_init: convert assert to return -errno hw/block/nvme.c| 5 +++- hw/misc/ivshmem.c | 8 +++--- hw/net/e1000e.c| 6 - hw/net/rocker/rocker.c | 7 - hw/net/vmxnet3.c | 46 ++-- hw/pci/msi.c | 11 +--- hw/pci/msix.c | 42 - hw/scsi/megasas.c | 49 +++--- hw/usb/hcd-xhci.c | 71 ++ hw/vfio/pci.c | 4 ++- hw/virtio/virtio-pci.c | 11 include/hw/pci/msix.h | 5 ++-- 12 files changed, 164 insertions(+), 101 deletions(-) -- 2.1.0
[Qemu-devel] [PATCH v5 07/10] megasas: undo the overwrites of msi user configuration
Commit afea4e14 seems forgetting to undo the overwrites, which is
unsuitable.
CC: Hannes Reinecke
CC: Paolo Bonzini
CC: Markus Armbruster
CC: Marcel Apfelbaum
CC: Michael S. Tsirkin
Reviewed-by: Markus Armbruster
Signed-off-by: Cao jin
---
hw/scsi/megasas.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index ba79e7a..bbef9e9 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -2337,11 +2337,10 @@ static void megasas_scsi_realize(PCIDevice *dev, Error
**errp)
"msi=off with this machine type.\n");
error_propagate(errp, err);
return;
-} else if (ret) {
-/* With msi=auto, we fall back to MSI off silently */
-s->msi = ON_OFF_AUTO_OFF;
-error_free(err);
}
+assert(!err || s->msix == ON_OFF_AUTO_AUTO);
+/* With msi=auto, we fall back to MSI off silently */
+error_free(err);
}
memory_region_init_io(&s->mmio_io, OBJECT(s), &megasas_mmio_ops, s,
--
2.1.0
Re: [Qemu-devel] [PULL 00/47] virtio, pc: fixes and features
On Wed, Nov 02, 2016 at 01:14:07PM +, Peter Maydell wrote: > On 2 November 2016 at 04:35, Michael S. Tsirkin wrote: > > On Tue, Nov 01, 2016 at 03:22:01PM +, Peter Maydell wrote: > >> On 30 October 2016 at 21:23, Michael S. Tsirkin wrote: > >> > The following changes since commit > >> > 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: > >> > > >> > Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161028-1' > >> > into staging (2016-10-28 17:59:04 +0100) > >> > > >> > are available in the git repository at: > >> > > >> > git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream > >> > > >> > for you to fetch changes up to f082ec0225bd15c71e0b4697d2df3af7bad65d7f: > >> > > >> > acpi: fix assert failure caused by commit 35c5a52d (2016-10-30 > >> > 20:06:25 +0200) > >> > > >> > > >> > virtio, pc: fixes and features > >> > > >> > nvdimm hotplug support > >> > virtio migration and ioeventfd rework > >> > virtio crypto device > >> > ipmi fixes > >> > > >> > Signed-off-by: Michael S. Tsirkin > >> > > >> > >> Hi; this fails to build on OSX with format string issues: > >> > >> /Users/pm215/src/qemu-for-merges/hw/virtio/virtio-crypto.c:770:20: > >> error: format specifies type 'unsign > >> ed short' but the argument has type 'uint32_t' (aka 'unsigned int') > >> [-Werror,-Wformat] > >>vcrypto->max_queues, VIRTIO_QUEUE_MAX); > >> ~~~^ > >> /Users/pm215/src/qemu-for-merges/include/qapi/error.h:163:35: note: > >> expanded from macro 'error_setg' > >> (fmt), ## __VA_ARGS__) > >> ^ > >> > >> Fun fact: in struct vhost_dev, max_queues is a uint64_t; > >> in struct VirtIONet it is a uint16_t; and in VirtIOCrypto > >> it is a uint32_t... > >> > >> thanks > >> -- PMM > > > > Just to make sure : I fixed that and pushed to > > same tag. I don't think it makes sense to repost the pull > > request - pls take it from > >git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream > > You should always repost at least the cover-letter part > of a fresh pull request, because otherwise it is likely > to not be caught by the email filters that find pull requests. > > (In this case I've handed over pull request processing > to Stefan, so the question would be whether his filters > notice.) > > thanks > -- PMM Stefan, could you confirm pls? -- MST
Re: [Qemu-devel] [PATCH] docs: Fix typos found by codespell
On 11/02/2016 09:29 PM, Eric Blake wrote: On 11/01/2016 08:32 PM, Zhang Chen wrote: I still don't understand the comment in docs/colo-proxy.txt. Me neither. Which part of the docs/colo-proxy.txt? The one quoted below. Perhaps I can explain it more clearly and make the doc better. +++ b/docs/colo-proxy.txt @@ -158,7 +158,7 @@ secondary. == Usage == -Here, we use demo ip and port discribe more clearly. +Here, we use demo ip and port describe more clearly. Primary(ip:3.3.3.3): -netdev tap,id=hn0,vhost=off,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown -device e1000,id=e0,netdev=hn0,mac=52:a4:00:12:78:66 Maybe: Here is an example using demonstration IP and port addresses to more clearly describe the usage. The remaining hunks are clear improvements, so: Reviewed-by: Eric Blake It's easier to understand. Okay, so you like my proposal for a potential reword. Now, who should submit it as a patch? Stefan (since he found the confusing sentence in the first place), you (since you wrote most of the document to begin with), or me (since I proposed wording that you like)? I will submit the patch later, if you think that's OK. -- Thanks zhangchen
[Qemu-devel] [PATCH] intel_iommu: fixing source id during IOTLB hash key calculation
Using uint8_t for source id will lose bus num and get the
wrong/invalid IOTLB entry. Fixing by using uint16_t instead and
enlarge level shift.
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc: Eduardo Habkost
Cc: Michael S. Tsirkin
Signed-off-by: Jason Wang
---
hw/i386/intel_iommu.c | 2 +-
hw/i386/intel_iommu_internal.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 2efd69b..1c5d40e 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -215,7 +215,7 @@ static void vtd_reset_iotlb(IntelIOMMUState *s)
g_hash_table_remove_all(s->iotlb);
}
-static uint64_t vtd_get_iotlb_key(uint64_t gfn, uint8_t source_id,
+static uint64_t vtd_get_iotlb_key(uint64_t gfn, uint16_t source_id,
uint32_t level)
{
return gfn | ((uint64_t)(source_id) << VTD_IOTLB_SID_SHIFT) |
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 0829a50..11abfa2 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -115,7 +115,7 @@
/* The shift of source_id in the key of IOTLB hash table */
#define VTD_IOTLB_SID_SHIFT 36
-#define VTD_IOTLB_LVL_SHIFT 44
+#define VTD_IOTLB_LVL_SHIFT 52
#define VTD_IOTLB_MAX_SIZE 1024/* Max size of the hash table */
/* IOTLB_REG */
--
2.7.4
[Qemu-devel] slirp: fix ipv6 guest network access with windows host
Hello qemu developers,
This patch is from android emulator (which is based on qemu2.2) and I
hope this patch is
also useful to upstream qemu as well.
bo
>From 021eac8c593a34a6a5e106d187a8e1fd22a1522f Mon Sep 17 00:00:00 2001
From: bohu
Date: Wed, 2 Nov 2016 15:56:26 -0700
Subject: [PATCH] slirp: fix ipv6 guest network access with windows host
In tcp_input function, local sockaddr_storage variables lhost
and fhost are used without being cleared to zero; and consequently
tcp connect call fails on windows because there is some random data
in those variables (windows complains with WSAEADDRNOTAVAIL);
This CL calls memset to clear those two variables so that the address
passed to connect does not have random data in it.
Signed-off-by: Bo Hu
---
slirp/tcp_input.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c
index c5063a9..9a79a16 100644
--- a/slirp/tcp_input.c
+++ b/slirp/tcp_input.c
@@ -234,6 +234,8 @@ tcp_input(struct mbuf *m, int iphlen, struct socket
*inso, unsigned short af)
struct sockaddr_in6 *lhost6, *fhost6;
struct ex_list *ex_ptr;
Slirp *slirp;
+memset(&lhost, 0, sizeof(lhost);
+memset(&fhost, 0, sizeof(fhost);
DEBUG_CALL("tcp_input");
DEBUG_ARGS((dfd, " m = %p iphlen = %2d inso = %p\n",
--
2.8.0.rc3.226.g39d4020
Re: [Qemu-devel] [Qemu-stable] [Qemu-ppc] [PULL 0/4] ppc patches for qemu-2.7 stable branch
Quoting David Gibson (2016-10-31 21:26:39) > On Tue, Oct 25, 2016 at 06:57:33PM -0500, Michael Roth wrote: > > Quoting David Gibson (2016-10-24 20:41:29) > > > On Mon, Oct 17, 2016 at 04:24:31PM -0500, Michael Roth wrote: > > > > Quoting Peter Maydell (2016-10-17 13:45:21) > > > > > On 17 October 2016 at 19:13, Michael Roth > > > > > wrote: > > > > > > We could do both though: use some ad-hoc way to tag for a particular > > > > > > sub-maintainer tree/stable branch, as well as an explicit "not for > > > > > > master" in the cover letter ensure it doesn't go into master. It's > > > > > > a bit > > > > > > more redundant, but flexible in that people can use whatever tagging > > > > > > format they want for a particular tree. > > > > > > > > > > Yes, that would be my preference. Gmail's filtering is not > > > > > very good, and it doesn't seem to be able to support > > > > > multiple or complex matches on the subject line, but > > > > > it can deal with "doesn't include foo in body". > > > > > People who actively want to look for stuff not to go > > > > > into master can filter it however they like. > > > > > > > > Sounds good to me. For my part I think "for-2.7.1" etc. would be > > > > prefereable. No need to resend this patchset though. > > > > > > > > I suppose MAINTAINERS would be the best place to document something > > > > like this? > > > > > > So.. regardless of the outcome in general for future stable merges.. > > > > > > Has this batch been merged for 2.7 stable? Or do I need to resend it > > > in the new style? > > > > No need to resend. I should have the initial staging tree for 2.7 posted > > by Monday and will have this included. > > I haven't spotted the 2.7 stable branch so far. Maybe I don't have > the right remote? Sorry, I was a bit behind getting it posted. I've put up a staging tree here: https://github.com/mdroth/qemu/commits/stable-2.7-staging I'm tentatively planning on posting the initial tree November 7th, setting the freeze for November 11th, and the release for the 16th. I saw your series regarding 2.6<->2.7 CPU migration and had also been hoping to get it sorted for 2.7.1, so let me know if we should consider tweaking the dates. > > -- > David Gibson| I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson
Re: [Qemu-devel] slirp: fix ipv6 guest network access with windows host
Hello, Bo Hu, on Wed 02 Nov 2016 16:02:29 -0700, wrote: > This patch is from android emulator (which is based on qemu2.2) and I hope > this patch is > also useful to upstream qemu as well. Indeed, even if normally we fill all required fields, it's probably better to memset the whole structure. > From 021eac8c593a34a6a5e106d187a8e1fd22a1522f Mon Sep 17 00:00:00 2001 > From: bohu <[1]b...@google.com> > Date: Wed, 2 Nov 2016 15:56:26 -0700 > Subject: [PATCH] slirp: fix ipv6 guest network access with windows host > > In tcp_input function, local sockaddr_storage variables lhost > and fhost are used without being cleared to zero; and consequently > tcp connect call fails on windows because there is some random data > in those variables (windows complains with WSAEADDRNOTAVAIL); > > This CL calls memset to clear those two variables so that the address > passed to connect does not have random data in it. > > Signed-off-by: Bo Hu <[2]b...@google.com> > + memset(&lhost, 0, sizeof(lhost); > + memset(&fhost, 0, sizeof(fhost); There is just a typo: missing closing parenthesis... But apart from that, Reviewed-by: Samuel Thibault Samuel
[Qemu-devel] [kvm-unit-tests PATCHv7 3/3] arm: pmu: Add CPI checking
Calculate the numbers of cycles per instruction (CPI) implied by ARM
PMU cycle counter values.
Signed-off-by: Christopher Covington
---
arm/pmu.c | 109 +-
1 file changed, 108 insertions(+), 1 deletion(-)
diff --git a/arm/pmu.c b/arm/pmu.c
index 65b7df1..ca00422 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -62,6 +62,23 @@ static inline void disable_counter(uint32_t idx)
{
asm volatile("mrc p15, 0, %0, c9, c12, 1" : : "r" (1 << idx));
}
+
+/*
+ * Extra instructions inserted by the compiler would be difficult to compensate
+ * for, so hand assemble everything between, and including, the PMCR accesses
+ * to start and stop counting.
+ */
+static inline void loop(int i, uint32_t pmcr)
+{
+ asm volatile(
+ " mcr p15, 0, %[pmcr], c9, c12, 0\n"
+ "1: subs%[i], %[i], #1\n"
+ " bgt 1b\n"
+ " mcr p15, 0, %[z], c9, c12, 0\n"
+ : [i] "+r" (i)
+ : [pmcr] "r" (pmcr), [z] "r" (0)
+ : "cc");
+}
#elif defined(__aarch64__)
static inline uint32_t get_pmcr(void)
{
@@ -98,6 +115,23 @@ static inline void disable_counter(uint32_t idx)
{
asm volatile("msr pmcntensclr_el0, %0" : : "r" (1 << idx));
}
+
+/*
+ * Extra instructions inserted by the compiler would be difficult to compensate
+ * for, so hand assemble everything between, and including, the PMCR accesses
+ * to start and stop counting.
+ */
+static inline void loop(int i, uint32_t pmcr)
+{
+ asm volatile(
+ " msr pmcr_el0, %[pmcr]\n"
+ "1: subs%[i], %[i], #1\n"
+ " b.gt1b\n"
+ " msr pmcr_el0, xzr\n"
+ : [i] "+r" (i)
+ : [pmcr] "r" (pmcr)
+ : "cc");
+}
#endif
struct pmu_data {
@@ -171,12 +205,85 @@ static bool check_cycles_increase(void)
return true;
}
-int main(void)
+/*
+ * Execute a known number of guest instructions. Only odd instruction counts
+ * greater than or equal to 3 are supported by the in-line assembly code. The
+ * control register (PMCR_EL0) is initialized with the provided value (allowing
+ * for example for the cycle counter or event counters to be reset). At the end
+ * of the exact instruction loop, zero is written to PMCR_EL0 to disable
+ * counting, allowing the cycle counter or event counters to be read at the
+ * leisure of the calling code.
+ */
+static void measure_instrs(int num, uint32_t pmcr)
+{
+ int i = (num - 1) / 2;
+
+ assert(num >= 3 && ((num - 1) % 2 == 0));
+ loop(i, pmcr);
+}
+
+/*
+ * Measure cycle counts for various known instruction counts. Ensure that the
+ * cycle counter progresses (similar to check_cycles_increase() but with more
+ * instructions and using reset and stop controls). If supplied a positive,
+ * nonzero CPI parameter, also strictly check that every measurement matches
+ * it. Strict CPI checking is used to test -icount mode.
+ */
+static bool check_cpi(int cpi)
+{
+ struct pmu_data pmu = {{0}};
+
+ enable_counter(ARMV8_PMU_CYCLE_IDX);
+ set_pmccfiltr(0); /* count cycles in EL0, EL1, but not EL2 */
+
+ pmu.cycle_counter_reset = 1;
+ pmu.enable = 1;
+
+ if (cpi > 0)
+ printf("Checking for CPI=%d.\n", cpi);
+ printf("instrs : cycles0 cycles1 ...\n");
+
+ for (int i = 3; i < 300; i += 32) {
+ int avg, sum = 0;
+
+ printf("%d :", i);
+ for (int j = 0; j < NR_SAMPLES; j++) {
+ int cycles;
+
+ measure_instrs(i, pmu.pmcr_el0);
+ cycles = get_pmccntr();
+ printf(" %d", cycles);
+
+ if (!cycles || (cpi > 0 && cycles != i * cpi)) {
+ printf("\n");
+ return false;
+ }
+
+ sum += cycles;
+ }
+ avg = sum / NR_SAMPLES;
+ printf(" sum=%d avg=%d avg_ipc=%d avg_cpi=%d\n",
+ sum, avg, i / avg, avg / i);
+ }
+
+ pmu.enable = 0;
+ set_pmcr(pmu.pmcr_el0);
+
+ return true;
+}
+
+int main(int argc, char *argv[])
{
+ int cpi = 0;
+
+ if (argc >= 1)
+ cpi = atol(argv[0]);
+
report_prefix_push("pmu");
report("Control register", check_pmcr());
report("Monotonically increasing cycle count", check_cycles_increase());
+ report("Cycle/instruction ratio", check_cpi(cpi));
return report_summary();
}
--
1.8.3.1
[Qemu-devel] [kvm-unit-tests PATCHv7 2/3] arm: pmu: Check cycle count increases
Ensure that reads of the PMCCNTR_EL0 are monotonically increasing,
even for the smallest delta of two subsequent reads.
Signed-off-by: Christopher Covington
Signed-off-by: Wei Huang
---
arm/pmu.c | 100 ++
1 file changed, 100 insertions(+)
diff --git a/arm/pmu.c b/arm/pmu.c
index 42d0ee1..65b7df1 100644
--- a/arm/pmu.c
+++ b/arm/pmu.c
@@ -14,6 +14,9 @@
*/
#include "libcflat.h"
+#define NR_SAMPLES 10
+#define ARMV8_PMU_CYCLE_IDX 31
+
#if defined(__arm__)
static inline uint32_t get_pmcr(void)
{
@@ -22,6 +25,43 @@ static inline uint32_t get_pmcr(void)
asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret));
return ret;
}
+
+static inline void set_pmcr(uint32_t pmcr)
+{
+ asm volatile("mcr p15, 0, %0, c9, c12, 0" : : "r" (pmcr));
+}
+
+static inline void set_pmccfiltr(uint32_t filter)
+{
+ uint32_t cycle_idx = ARMV8_PMU_CYCLE_IDX;
+
+ asm volatile("mcr p15, 0, %0, c9, c12, 5" : : "r" (cycle_idx));
+ asm volatile("mcr p15, 0, %0, c9, c13, 1" : : "r" (filter));
+}
+
+/*
+ * While PMCCNTR can be accessed as a 64 bit coprocessor register, returning 64
+ * bits doesn't seem worth the trouble when differential usage of the result is
+ * expected (with differences that can easily fit in 32 bits). So just return
+ * the lower 32 bits of the cycle count in AArch32.
+ */
+static inline unsigned long get_pmccntr(void)
+{
+ unsigned long cycles;
+
+ asm volatile("mrc p15, 0, %0, c9, c13, 0" : "=r" (cycles));
+ return cycles;
+}
+
+static inline void enable_counter(uint32_t idx)
+{
+ asm volatile("mcr p15, 0, %0, c9, c12, 1" : : "r" (1 << idx));
+}
+
+static inline void disable_counter(uint32_t idx)
+{
+ asm volatile("mrc p15, 0, %0, c9, c12, 1" : : "r" (1 << idx));
+}
#elif defined(__aarch64__)
static inline uint32_t get_pmcr(void)
{
@@ -30,6 +70,34 @@ static inline uint32_t get_pmcr(void)
asm volatile("mrs %0, pmcr_el0" : "=r" (ret));
return ret;
}
+
+static inline void set_pmcr(uint32_t pmcr)
+{
+ asm volatile("msr pmcr_el0, %0" : : "r" (pmcr));
+}
+
+static inline void set_pmccfiltr(uint32_t filter)
+{
+ asm volatile("msr pmccfiltr_el0, %0" : : "r" (filter));
+}
+
+static inline unsigned long get_pmccntr(void)
+{
+ unsigned long cycles;
+
+ asm volatile("mrs %0, pmccntr_el0" : "=r" (cycles));
+ return cycles;
+}
+
+static inline void enable_counter(uint32_t idx)
+{
+ asm volatile("msr pmcntenset_el0, %0" : : "r" (1 << idx));
+}
+
+static inline void disable_counter(uint32_t idx)
+{
+ asm volatile("msr pmcntensclr_el0, %0" : : "r" (1 << idx));
+}
#endif
struct pmu_data {
@@ -72,11 +140,43 @@ static bool check_pmcr(void)
return pmu.implementer != 0;
}
+/*
+ * Ensure that the cycle counter progresses between back-to-back reads.
+ */
+static bool check_cycles_increase(void)
+{
+ struct pmu_data pmu = {{0}};
+
+ enable_counter(ARMV8_PMU_CYCLE_IDX);
+ set_pmccfiltr(0); /* count cycles in EL0, EL1, but not EL2 */
+
+ pmu.enable = 1;
+ set_pmcr(pmu.pmcr_el0);
+
+ for (int i = 0; i < NR_SAMPLES; i++) {
+ unsigned long a, b;
+
+ a = get_pmccntr();
+ b = get_pmccntr();
+
+ if (a >= b) {
+ printf("Read %ld then %ld.\n", a, b);
+ return false;
+ }
+ }
+
+ pmu.enable = 0;
+ set_pmcr(pmu.pmcr_el0);
+
+ return true;
+}
+
int main(void)
{
report_prefix_push("pmu");
report("Control register", check_pmcr());
+ report("Monotonically increasing cycle count", check_cycles_increase());
return report_summary();
}
--
1.8.3.1
[Qemu-devel] [kvm-unit-tests PATCHv7 0/3] ARM PMU tests
Changes from v6: * Add a new pmu testing for KVM mode in config file * Add additional init code, including setting PMCNTENSET and PMCCFILTR, before reading PMCCNTR. ARMv7 support is also provided * Add cycle counter init code for CPI testing * Fix pmu_data compilation issue (for gcc 4.8.5) * Commit comments were updated NOTE: 1) Current KVM code has bugs in handling PMCCFILTR write. A fix (see below) is required for this unit testing code to work correctly under KVM mode. https://lists.cs.columbia.edu/pipermail/kvmarm/2016-November/022134.html. 2) Because the code was changed, Drew's original reviewed-by needs to be acknowledged by him again.
[Qemu-devel] [kvm-unit-tests PATCHv7 1/3] arm: Add PMU test
Beginning with a simple sanity check of the control register, add
a unit test for the ARM Performance Monitors Unit (PMU).
Signed-off-by: Christopher Covington
---
arm/Makefile.common | 3 +-
arm/pmu.c | 82 +
arm/unittests.cfg | 20 +
3 files changed, 104 insertions(+), 1 deletion(-)
create mode 100644 arm/pmu.c
diff --git a/arm/Makefile.common b/arm/Makefile.common
index ccb554d..f98f422 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -11,7 +11,8 @@ endif
tests-common = \
$(TEST_DIR)/selftest.flat \
- $(TEST_DIR)/spinlock-test.flat
+ $(TEST_DIR)/spinlock-test.flat \
+ $(TEST_DIR)/pmu.flat
all: test_cases
diff --git a/arm/pmu.c b/arm/pmu.c
new file mode 100644
index 000..42d0ee1
--- /dev/null
+++ b/arm/pmu.c
@@ -0,0 +1,82 @@
+/*
+ * Test the ARM Performance Monitors Unit (PMU).
+ *
+ * Copyright 2015 The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License version 2.1 and
+ * only version 2.1 as published by the Free Software Foundation.
+ *
+ * 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 Lesser General Public License
+ * for more details.
+ */
+#include "libcflat.h"
+
+#if defined(__arm__)
+static inline uint32_t get_pmcr(void)
+{
+ uint32_t ret;
+
+ asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (ret));
+ return ret;
+}
+#elif defined(__aarch64__)
+static inline uint32_t get_pmcr(void)
+{
+ uint32_t ret;
+
+ asm volatile("mrs %0, pmcr_el0" : "=r" (ret));
+ return ret;
+}
+#endif
+
+struct pmu_data {
+ union {
+ uint32_t pmcr_el0;
+ struct {
+ uint32_t enable:1;
+ uint32_t event_counter_reset:1;
+ uint32_t cycle_counter_reset:1;
+ uint32_t cycle_counter_clock_divider:1;
+ uint32_t event_counter_export:1;
+ uint32_t cycle_counter_disable_when_prohibited:1;
+ uint32_t cycle_counter_long:1;
+ uint32_t reserved:4;
+ uint32_t counters:5;
+ uint32_t identification_code:8;
+ uint32_t implementer:8;
+ };
+ };
+};
+
+/*
+ * As a simple sanity check on the PMCR_EL0, ensure the implementer field isn't
+ * null. Also print out a couple other interesting fields for diagnostic
+ * purposes. For example, as of fall 2015, QEMU TCG mode doesn't implement
+ * event counters and therefore reports zero event counters, but hopefully
+ * support for at least the instructions event will be added in the future and
+ * the reported number of event counters will become nonzero.
+ */
+static bool check_pmcr(void)
+{
+ struct pmu_data pmu;
+
+ pmu.pmcr_el0 = get_pmcr();
+
+ printf("PMU implementer: %c\n", pmu.implementer);
+ printf("Identification code: 0x%x\n", pmu.identification_code);
+ printf("Event counters: %d\n", pmu.counters);
+
+ return pmu.implementer != 0;
+}
+
+int main(void)
+{
+ report_prefix_push("pmu");
+
+ report("Control register", check_pmcr());
+
+ return report_summary();
+}
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 3f6fa45..b647b69 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -54,3 +54,23 @@ file = selftest.flat
smp = $MAX_SMP
extra_params = -append 'smp'
groups = selftest
+
+# Test PMU support (KVM)
+[pmu-kvm]
+file = pmu.flat
+groups = pmu
+accel = kvm
+
+# Test PMU support (TCG) with -icount IPC=1
+[pmu-tcg-icount-1]
+file = pmu.flat
+extra_params = -icount 0 -append '1'
+groups = pmu
+accel = tcg
+
+# Test PMU support (TCG) with -icount IPC=256
+[pmu-tcg-icount-256]
+file = pmu.flat
+extra_params = -icount 8 -append '256'
+groups = pmu
+accel = tcg
--
1.8.3.1
Re: [Qemu-devel] [Bug 1636217] Re: qemu-kvm 2.7 does not boot kvm VMs with virtio on top of VMware ESX
Adding Gerd, Marcel, and Kevin On 10/28/16 10:23, Fabian Grünbichler wrote: > I traced this back to the switch to enabling virtio-1 mode by default in > 2.7 in commit 9a4c0e220d8a4f82b5665d0ee95ef94d8e1509d5 > > forcing the old behaviour with a 2.6 machine type works. I think this issue is a duplicate of the following RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1373154 and the *SeaBIOS* commit that makes it all work again is: commit 0e21548b15e25e010c362ea13d170c61a3fcc899 Author: Gerd Hoffmann Date: Fri Jul 3 11:07:05 2015 +0200 virtio: pci cfg access That SeaBIOS commit is part of the SeaBIOS 1.10.0 release. However, QEMU 2.7 shipped with bundled SeaBIOS 1.9.3 binaries. See QEMU commits 6e03a28e1cee (part of v2.7.0) and 6e99f5741ff1 (not part of any tagged release yet). The fix is probably the following: - backport SeaBIOS commit 0e21548b15e2 to the stable 1.9 branch, for release 1.9.4 - bundle SeaBIOS 1.9.4 binaries with QEMU v2.7.1. Thanks Laszlo
Re: [Qemu-devel] when we add EL2 to QEMU TCG ARM emulation and the virt board, should it default on or off?
On 11/01/16 18:16, Peter Maydell wrote: > I'm working on turning on EL2 support in our TCG ARM emulation, > and one area I'm not sure about is whether it should default to > on or off. > > We have a few precedents: > > For EL3 support: > * the CPU property is enabled by default but can be disabled by the board > * 'virt' defaults it to disabled, with a -machine secure=on property >which turns it on (barfing if KVM is enabled) and also does some other >stuff like wiring up secure-only memory and devices and making sure >the GIC has security enabled > * if the user does -cpu has_el3=yes things will probably not go >too well and that's unsupported > > For PMU support: > * the CPU property is enabled by default > * 'virt' defaults it to enabled (except for virt-2.6 and earlier) > * we do expect the user to be able to turn it on and off with >a -cpu pmu=yes/no option (and the board model changes behaviour >based on whether the CPU claims to have the feature) > > So what about EL2? Some background facts that are probably relevant: > * at the moment KVM can't support it, but eventually machines with >the nested virtualization hardware support will appear (this is >an ARMv8.3 feature), at which point it ought to work there too > * if you just enable EL2 then some currently working setups stop >working (basically any code that was written to assume it was >entered in EL1); notably this includes kvm-unit-tests (patches >pending from Drew that fix that). Linux is fine though as it >checks and handles both. > > Disabled-by-default has the advantage that (a) a command line > will work the same for TCG and KVM Definitely a bonus in my book. > and (b) nothing that used to > work will break. I consider this a requirement, sort of. Regressions are very very annoying (unless we can prove the previous command line was bogus to begin with, and just happened to work -- I don't think that applies in this case). > The disadvantage is that anybody who wants to > be able to run nested KVM will now have to specify a command line > option of some kind. Nested KVM is sophisticated / non-standard enough that one more tweak for utilizing it shouldn't be a problem. For example, considering the kvm_intel host module (yes, it's x86, but the parallel is obvious), it has an explicit parameter called "nested", which defaults to N. arch/x86/kvm/vmx.c: /* * If nested=1, nested virtualization is supported, i.e., guests may use * VMX and be a hypervisor for its own guests. If nested=0, guests may not * use VMX instructions. */ static bool __read_mostly nested = 0; module_param(nested, bool, S_IRUGO); Additionally, even with modprobe kvm_intel nested=1 I think the explicit setting -cpu MODEL,+vmx remains necessary. Thus, "disabled by default", please. Thanks Laszlo > > So: > (1) should we default on or off? (both at the board level, > and for the cpu object itself) > (2) directly user-set cpu property, or board property ? > > thanks > -- PMM >
[Qemu-devel] [PATCH v2 3/3] target-m68k: add cas/cas2 ops
Implement CAS using cmpxchg.
Implement CAS2 using helper and either cmpxchg when
the 32bit addresses are consecutive, or with
parallel_cpus+cpu_loop_exit_atomic() otherwise.
Suggested-by: Richard Henderson
Signed-off-by: Laurent Vivier
---
target-m68k/helper.h| 2 +
target-m68k/op_helper.c | 133
target-m68k/translate.c | 143
3 files changed, 278 insertions(+)
diff --git a/target-m68k/helper.h b/target-m68k/helper.h
index d863e55..17ec342 100644
--- a/target-m68k/helper.h
+++ b/target-m68k/helper.h
@@ -9,6 +9,8 @@ DEF_HELPER_4(divull, void, env, int, int, i32)
DEF_HELPER_4(divsll, void, env, int, int, s32)
DEF_HELPER_2(set_sr, void, env, i32)
DEF_HELPER_3(movec, void, env, i32, i32)
+DEF_HELPER_4(cas2w, void, env, i32, i32, i32)
+DEF_HELPER_4(cas2l, void, env, i32, i32, i32)
DEF_HELPER_2(f64_to_i32, f32, env, f64)
DEF_HELPER_2(f64_to_f32, f32, env, f64)
diff --git a/target-m68k/op_helper.c b/target-m68k/op_helper.c
index a4bfa4e..ff27211 100644
--- a/target-m68k/op_helper.c
+++ b/target-m68k/op_helper.c
@@ -359,3 +359,136 @@ void HELPER(divsll)(CPUM68KState *env, int numr, int
regr, int32_t den)
env->dregs[regr] = rem;
env->dregs[numr] = quot;
}
+
+void HELPER(cas2w)(CPUM68KState *env, uint32_t regs, uint32_t a1, uint32_t a2)
+{
+uint32_t Dc1 = extract32(regs, 9, 3);
+uint32_t Dc2 = extract32(regs, 6, 3);
+uint32_t Du1 = extract32(regs, 3, 3);
+uint32_t Du2 = extract32(regs, 0, 3);
+int16_t c1 = env->dregs[Dc1];
+int16_t c2 = env->dregs[Dc2];
+int16_t u1 = env->dregs[Du1];
+int16_t u2 = env->dregs[Du2];
+int16_t l1, l2;
+uintptr_t ra = GETPC();
+
+if (parallel_cpus) {
+/* Tell the main loop we need to serialize this insn. */
+cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
+} else {
+/* We're executing in a serial context -- no need to be atomic. */
+#ifdef CONFIG_USER_ONLY
+int16_t *ha1 = g2h(a1);
+int16_t *ha2 = g2h(a2);
+l1 = ldsw_be_p(ha1);
+l2 = ldsw_be_p(ha2);
+if (l1 == c1 && l2 == c2) {
+stw_be_p(ha1, u1);
+stw_be_p(ha2, u2);
+}
+#else
+int mmu_idx = cpu_mmu_index(env, 0);
+TCGMemOpIdx oi = make_memop_idx(MO_BEUW, mmu_idx);
+l1 = helper_be_ldsw_mmu(env, a1, oi, ra);
+l2 = helper_be_ldsw_mmu(env, a2, oi, ra);
+if (l1 == c1 && l2 == c2) {
+helper_be_stw_mmu(env, a1, u1, oi, ra);
+helper_be_stw_mmu(env, a2, u2, oi, ra);
+}
+#endif
+}
+
+if (c1 != l1) {
+env->cc_n = l1;
+env->cc_v = c1;
+} else {
+env->cc_n = l2;
+env->cc_v = c2;
+}
+env->cc_op = CC_OP_CMPL;
+env->dregs[Dc1] = deposit32(env->dregs[Dc1], 0, 16, l1);
+env->dregs[Dc2] = deposit32(env->dregs[Dc2], 0, 16, l2);
+}
+
+void HELPER(cas2l)(CPUM68KState *env, uint32_t regs, uint32_t a1, uint32_t a2)
+{
+uint32_t Dc1 = extract32(regs, 9, 3);
+uint32_t Dc2 = extract32(regs, 6, 3);
+uint32_t Du1 = extract32(regs, 3, 3);
+uint32_t Du2 = extract32(regs, 0, 3);
+uint32_t c1 = env->dregs[Dc1];
+uint32_t c2 = env->dregs[Dc2];
+uint32_t u1 = env->dregs[Du1];
+uint32_t u2 = env->dregs[Du2];
+uint32_t l1, l2;
+uint64_t c, u, l;
+uintptr_t ra = GETPC();
+#ifndef CONFIG_USER_ONLY
+int mmu_idx = cpu_mmu_index(env, 0);
+TCGMemOpIdx oi;
+#endif
+
+if (parallel_cpus) {
+/* We're executing in a parallel context -- must be atomic. */
+if ((a1 & 7) == 0 && a2 == a1 + 4) {
+c = deposit64(c2, 32, 32, c1);
+u = deposit64(u2, 32, 32, u1);
+#ifdef CONFIG_USER_ONLY
+uint64_t *ha1 = g2h(a1);
+l = atomic_cmpxchg__nocheck(ha1, c, u);
+#else
+oi = make_memop_idx(MO_BEQ, mmu_idx);
+l = helper_atomic_cmpxchgq_be_mmu(env, a1, c, u, oi, ra);
+#endif
+l1 = l >> 32;
+l2 = l;
+} else if ((a2 & 7) == 0 && a1 == a2 + 4) {
+c = deposit64(c1, 32, 32, c2);
+u = deposit64(u1, 32, 32, u2);
+#ifdef CONFIG_USER_ONLY
+uint64_t *ha1 = g2h(a1);
+l = atomic_cmpxchg__nocheck(ha1, c, u);
+#else
+oi = make_memop_idx(MO_BEQ, mmu_idx);
+l = helper_atomic_cmpxchgq_be_mmu(env, a1, c, u, oi, ra);
+#endif
+l2 = l >> 32;
+l1 = l;
+} else {
+/* Tell the main loop we need to serialize this insn. */
+cpu_loop_exit_atomic(ENV_GET_CPU(env), ra);
+}
+} else {
+#ifdef CONFIG_USER_ONLY
+uint32_t *ha1 = g2h(a1);
+uint32_t *ha2 = g2h(a2);
+l1 = ldl_be_p(ha1);
+l2 = ldl_be_p(ha2);
+if (l1 == c1 && l2 == c2) {
+stl_be_p(ha1, u1);
+stl_be_p(ha2, u2);
+}
+#else
+/* We're executing in a serial context -- no need to be atomic. */
+
[Qemu-devel] [PATCH v2 1/3] target-m68k: add abcd/sbcd/nbcd
Signed-off-by: Laurent Vivier
---
target-m68k/translate.c | 242
1 file changed, 242 insertions(+)
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index d0a3b0f..1cf88a4 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1313,6 +1313,243 @@ DISAS_INSN(divl)
set_cc_op(s, CC_OP_FLAGS);
}
+static void bcd_add(TCGv dest, TCGv src)
+{
+TCGv t0, t1;
+
+/* dest10 = dest10 + src10 + X
+ *
+ *t1 = src
+ *t2 = t1 + 0x066
+ *t3 = t2 + dest + X
+ *t4 = t2 ^ dest ^ X
+ *t5 = t3 ^ t4
+ *t6 = ~t5 & 0x110
+ *t7 = (t6 >> 2) | (t6 >> 3)
+ *return t3 - t7
+ */
+
+/* t1 = (src + 0x066) + dest + X
+ *= result with some possible exceding 0x6
+ */
+
+t0 = tcg_const_i32(0x066);
+tcg_gen_add_i32(t0, t0, src);
+
+t1 = tcg_temp_new();
+tcg_gen_add_i32(t1, t0, dest);
+tcg_gen_add_i32(t1, t1, QREG_CC_X);
+
+/* we will remove exceding 0x6 where there is no carry */
+
+/* t0 = (src + 0x0066) ^ dest ^ X
+ *= t1 without carries
+ */
+
+tcg_gen_xor_i32(t0, t0, dest);
+tcg_gen_xor_i32(t0, t0, QREG_CC_X);
+
+/* extract the carries
+ * t0 = t0 ^ t1
+ *= only the carries
+ */
+
+tcg_gen_xor_i32(t0, t0, t1);
+
+/* generate 0x1 where there is no carry */
+
+tcg_gen_not_i32(t0, t0);
+tcg_gen_andi_i32(t0, t0, 0x110);
+
+/* for each 0x10, generate a 0x6 */
+
+tcg_gen_shri_i32(dest, t0, 2);
+tcg_gen_shri_i32(t0, t0, 3);
+tcg_gen_or_i32(dest, dest, t0);
+tcg_temp_free(t0);
+
+/* remove the exceding 0x6
+ * for digits that have not generated a carry
+ */
+
+tcg_gen_sub_i32(dest, t1, dest);
+tcg_temp_free(t1);
+}
+
+static void bcd_sub(TCGv dest, TCGv src)
+{
+TCGv t0, t1, t2;
+
+/* dest10 = dest10 - src10 - X
+ * = bcd_add(dest + 1 - X, 0xf99 - src)
+ */
+
+/* t0 = 0xfff - src */
+
+t0 = tcg_temp_new();
+tcg_gen_neg_i32(t0, src);
+tcg_gen_addi_i32(t0, t0, 0xfff);
+
+/* t1 = t0 + dest + 1 - X*/
+
+t1 = tcg_temp_new();
+tcg_gen_add_i32(t1, t0, dest);
+tcg_gen_addi_i32(t1, t1, 1);
+tcg_gen_sub_i32(t1, t1, QREG_CC_X);
+
+/* t2 = t0 ^ dest ^ 1 ^ X */
+
+t2 = tcg_temp_new();
+tcg_gen_xor_i32(t2, t0, dest);
+tcg_gen_xori_i32(t2, t2, 1);
+tcg_gen_xor_i32(t2, t2, QREG_CC_X);
+
+/* t0 = t1 ^ t2 */
+
+tcg_gen_xor_i32(t0, t1, t2);
+
+/* t2 = ~t0 & 0x110 */
+
+tcg_gen_not_i32(t2, t0);
+tcg_gen_andi_i32(t2, t2, 0x110);
+
+/* t0 = (t2 >> 2) | (t2 >> 3) */
+
+tcg_gen_shri_i32(t0, t2, 2);
+tcg_gen_shri_i32(t2, t2, 3);
+tcg_gen_or_i32(t0, t0, t2);
+
+/* return t1 - t0 */
+
+tcg_gen_sub_i32(dest, t1, t0);
+}
+
+static void bcd_flags(TCGv val)
+{
+tcg_gen_andi_i32(QREG_CC_C, val, 0x0ff);
+tcg_gen_or_i32(QREG_CC_Z, QREG_CC_Z, QREG_CC_C);
+
+tcg_gen_movi_i32(QREG_CC_X, 0);
+tcg_gen_andi_i32(val, val, 0xf00);
+tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_C, val, QREG_CC_X);
+
+tcg_gen_mov_i32(QREG_CC_X, QREG_CC_C);
+}
+
+DISAS_INSN(abcd_reg)
+{
+TCGv src;
+TCGv dest;
+
+gen_flush_flags(s); /* !Z is sticky */
+
+src = gen_extend(DREG(insn, 0), OS_BYTE, 0);
+dest = gen_extend(DREG(insn, 9), OS_BYTE, 0);
+bcd_add(dest, src);
+gen_partset_reg(OS_BYTE, DREG(insn, 9), dest);
+
+bcd_flags(dest);
+}
+
+DISAS_INSN(abcd_mem)
+{
+TCGv src;
+TCGv addr_src;
+TCGv dest;
+TCGv addr_dest;
+
+gen_flush_flags(s); /* !Z is sticky */
+
+addr_src = tcg_temp_new();
+tcg_gen_subi_i32(addr_src, AREG(insn, 0), opsize_bytes(OS_BYTE));
+src = gen_load(s, OS_BYTE, addr_src, 0);
+
+addr_dest = tcg_temp_new();
+if (REG(insn, 0) == REG(insn, 9)) {
+tcg_gen_subi_i32(addr_dest, addr_src, opsize_bytes(OS_BYTE));
+} else {
+tcg_gen_subi_i32(addr_dest, AREG(insn, 9), opsize_bytes(OS_BYTE));
+}
+dest = gen_load(s, OS_BYTE, addr_dest, 0);
+
+bcd_add(dest, src);
+
+gen_store(s, OS_BYTE, addr_dest, dest);
+
+tcg_gen_mov_i32(AREG(insn, 0), addr_src);
+tcg_temp_free(addr_src);
+tcg_gen_mov_i32(AREG(insn, 9), addr_dest);
+tcg_temp_free(addr_dest);
+
+bcd_flags(dest);
+}
+
+DISAS_INSN(sbcd_reg)
+{
+TCGv src, dest;
+
+gen_flush_flags(s); /* !Z is sticky */
+
+src = gen_extend(DREG(insn, 0), OS_BYTE, 0);
+dest = gen_extend(DREG(insn, 9), OS_BYTE, 0);
+
+bcd_sub(dest, src);
+
+gen_partset_reg(OS_BYTE, DREG(insn, 9), dest);
+
+bcd_flags(dest);
+}
+
+DISAS_INSN(sbcd_mem)
+{
+TCGv src, dest;
+TCGv addr_src, addr_dest;
+
+gen_flush_flags(s); /* !Z is sticky */
+
+addr_src = tcg_temp_new();
+tcg_gen_subi_i32(addr_src, AREG(insn, 0), opsize_bytes(OS_BYTE));
+src = gen_load(s, OS_BYTE, addr_src, 0);
+
+addr_dest = tcg_temp_new();
+if (REG(insn, 0) == REG(insn
[Qemu-devel] [PATCH v2 2/3] target-m68k: implement 680x0 movem
680x0 movem can load/store words and long words
and can use more addressing modes.
Coldfire can only use long words with (Ax) and (d16,Ax)
addressing modes.
Signed-off-by: Laurent Vivier
---
target-m68k/translate.c | 96 -
1 file changed, 79 insertions(+), 17 deletions(-)
diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 1cf88a4..93f1270 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1667,14 +1667,25 @@ static void gen_push(DisasContext *s, TCGv val)
tcg_gen_mov_i32(QREG_SP, tmp);
}
+static TCGv mreg(int reg)
+{
+if (reg < 8) {
+/* Dx */
+return cpu_dregs[reg];
+}
+/* Ax */
+return cpu_aregs[reg & 7];
+}
+
DISAS_INSN(movem)
{
TCGv addr;
int i;
uint16_t mask;
-TCGv reg;
TCGv tmp;
-int is_load;
+int is_load = (insn & 0x0400) != 0;
+int opsize = (insn & 0x40) != 0 ? OS_LONG : OS_WORD;
+TCGv incr;
mask = read_im16(env, s);
tmp = gen_lea(env, s, insn, OS_LONG);
@@ -1682,25 +1693,74 @@ DISAS_INSN(movem)
gen_addr_fault(s);
return;
}
+
addr = tcg_temp_new();
tcg_gen_mov_i32(addr, tmp);
-is_load = ((insn & 0x0400) != 0);
-for (i = 0; i < 16; i++, mask >>= 1) {
-if (mask & 1) {
-if (i < 8)
-reg = DREG(i, 0);
-else
-reg = AREG(i, 0);
-if (is_load) {
-tmp = gen_load(s, OS_LONG, addr, 0);
-tcg_gen_mov_i32(reg, tmp);
-} else {
-gen_store(s, OS_LONG, addr, reg);
+incr = tcg_const_i32(opsize_bytes(opsize));
+
+if (is_load) {
+/* memory to register */
+TCGv r[16];
+for (i = 0; i < 16; i++) {
+if ((mask >> i) & 1) {
+r[i] = gen_load(s, opsize, addr, 1);
+tcg_gen_add_i32(addr, addr, incr);
+}
+}
+for (i = 0; i < 16; i++, mask >>= 1) {
+if (mask & 1) {
+tcg_gen_mov_i32(mreg(i), r[i]);
+tcg_temp_free(r[i]);
+}
+}
+if ((insn & 070) == 030) {
+/* movem (An)+,X */
+tcg_gen_mov_i32(AREG(insn, 0), addr);
+}
+
+} else {
+/* register to memory */
+
+if ((insn & 070) == 040) {
+/* movem X,-(An) */
+
+for (i = 15; i >= 0; i--, mask >>= 1) {
+if (mask & 1) {
+if ((insn & 7) + 8 == i &&
+m68k_feature(s->env, M68K_FEATURE_EXT_FULL)) {
+/* M68020+: if the addressing register is the
+ * register moved to memory, the value written
+ * is the initial value decremented by the size of
+ * the operation
+ * M68000/M68010: the value is the initial value
+ */
+TCGv tmp = tcg_temp_new();
+tcg_gen_sub_i32(tmp, mreg(i), incr);
+gen_store(s, opsize, addr, tmp);
+tcg_temp_free(tmp);
+} else {
+gen_store(s, opsize, addr, mreg(i));
+}
+if (mask != 1) {
+tcg_gen_sub_i32(addr, addr, incr);
+}
+}
+}
+tcg_gen_mov_i32(AREG(insn, 0), addr);
+} else {
+/* movem X,(An)+ is not allowed */
+
+for (i = 0; i < 16; i++, mask >>= 1) {
+if (mask & 1) {
+gen_store(s, opsize, addr, mreg(i));
+tcg_gen_add_i32(addr, addr, incr);
+}
}
-if (mask != 1)
-tcg_gen_addi_i32(addr, addr, 4);
}
}
+
+tcg_temp_free(incr);
+tcg_temp_free(addr);
}
DISAS_INSN(bitop_im)
@@ -3858,7 +3918,9 @@ void register_m68k_insns (CPUM68KState *env)
BASE(pea, 4840, ffc0);
BASE(swap, 4840, fff8);
INSN(bkpt, 4848, fff8, BKPT);
-BASE(movem, 48c0, fbc0);
+INSN(movem, 48d0, fbf8, CF_ISA_A);
+INSN(movem, 48e8, fbf8, CF_ISA_A);
+INSN(movem, 4880, fb80, M68000);
BASE(ext, 4880, fff8);
BASE(ext, 48c0, fff8);
BASE(ext, 49c0, fff8);
--
2.7.4
[Qemu-devel] [PATCH v2 0/3] target-m68k: add movem, BCD and CAS instructions
This series is another subset of the series I sent in May: https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg00501.html This subset contains reworked patches for: - abcd/nbcd/sbcd: remove inline, delay write back to memory and use only 3 digits (and extract it from the bifield patch as it was squashed into it) - movem: delay the update of the registers to the end of the load sequence to be able to restart the operation in case of page fault, and manage the 68020+ <-> 68000/68010 special case - cas/cas2: rewrite according Richard's comments, and use cmpxchg() in cas. I've checked it doesn't break coldfire support: http://wiki.qemu.org/download/coldfire-test-0.1.tar.bz2 but it can't boot a 680x0 processor kernel. v2: - movem: don't use temp variable mask2 - bcd: delay register write back fix bcd_add() to add X define bcd_sub() instead of bsd_neg() - cas2: make it atomic with "parallel_cpus" and cpu_loop_exit_atomic() (and 64bit cmpxchg() if possible) Richard: This series doesn't use your "areg writeback" series, but if you resend your series (with cmpm patches squashed and the fix for writeback_mask), I will rebase this series on top of yours. Thanks, Laurent Laurent Vivier (3): target-m68k: add abcd/sbcd/nbcd target-m68k: implement 680x0 movem target-m68k: add cas/cas2 ops target-m68k/helper.h| 2 + target-m68k/op_helper.c | 133 + target-m68k/translate.c | 481 ++-- 3 files changed, 599 insertions(+), 17 deletions(-) -- 2.7.4
[Qemu-devel] [PULL 0/2] tags/xen-20161102-tag
The following changes since commit 4eb28abd52d48657cff6ff45e8dbbbefe4dbb414: Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20161101-2' into staging (2016-11-01 16:53:05 +) are available in the git repository at: git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/xen-20161102-tag for you to fetch changes up to 021746c131cdfeab9d82ff918795a9f18d20d7ae: PCMachineState: introduce acpi_build_enabled field (2016-11-02 12:26:12 -0700) Xen 2016/11/02 Thomas Huth (1): hw/xen/xen_pvdev: Include qemu/log.h for qemu_log_vprintf() Wei Liu (1): PCMachineState: introduce acpi_build_enabled field hw/i386/acpi-build.c | 2 +- hw/i386/pc.c | 2 ++ hw/xen/xen_pvdev.c | 2 +- include/hw/i386/pc.h | 2 ++ xen-common.c | 6 ++ 5 files changed, 12 insertions(+), 2 deletions(-)
[Qemu-devel] [PULL 2/2] PCMachineState: introduce acpi_build_enabled field
From: Wei Liu
Introduce this field to control whether ACPI build is enabled by a
particular machine or accelerator.
It defaults to true if the machine itself supports ACPI build. Xen
accelerator will disable it because Xen is in charge of building ACPI
tables for the guest.
Signed-off-by: Wei Liu
Signed-off-by: Stefano Stabellini
Reviewed-by: Stefano Stabellini
Reviewed-by: Eduardo Habkost
Tested-by: Sander Eikelenboom
---
hw/i386/acpi-build.c | 2 +-
hw/i386/pc.c | 2 ++
include/hw/i386/pc.h | 2 ++
xen-common.c | 6 ++
4 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5cd1da9..13cbbde 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2953,7 +2953,7 @@ void acpi_setup(void)
return;
}
-if (!pcmc->has_acpi_build) {
+if (!pcms->acpi_build_enabled) {
ACPI_BUILD_DPRINTF("ACPI build disabled. Bailing out.\n");
return;
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f56ea0f..fbd9aed 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2159,6 +2159,8 @@ static void pc_machine_initfn(Object *obj)
pcms->vmport = ON_OFF_AUTO_AUTO;
/* nvdimm is disabled on default. */
pcms->acpi_nvdimm_state.is_enabled = false;
+/* acpi build is enabled by default if machine supports it */
+pcms->acpi_build_enabled = PC_MACHINE_GET_CLASS(pcms)->has_acpi_build;
}
static void pc_machine_reset(void)
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 98dc772..8eb517f 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -62,6 +62,8 @@ struct PCMachineState {
AcpiNVDIMMState acpi_nvdimm_state;
+bool acpi_build_enabled;
+
/* RAM information (sizes, addresses, configuration): */
ram_addr_t below_4g_mem_size, above_4g_mem_size;
diff --git a/xen-common.c b/xen-common.c
index 9099760..bacf962 100644
--- a/xen-common.c
+++ b/xen-common.c
@@ -9,6 +9,7 @@
*/
#include "qemu/osdep.h"
+#include "hw/i386/pc.h"
#include "hw/xen/xen_backend.h"
#include "qmp-commands.h"
#include "sysemu/char.h"
@@ -114,6 +115,11 @@ static void xen_change_state_handler(void *opaque, int
running,
static int xen_init(MachineState *ms)
{
+PCMachineState *pcms = PC_MACHINE(ms);
+
+/* Disable ACPI build because Xen handles it */
+pcms->acpi_build_enabled = false;
+
xen_xc = xc_interface_open(0, 0, 0);
if (xen_xc == NULL) {
xen_pv_printf(NULL, 0, "can't open xen interface\n");
--
1.9.1
[Qemu-devel] [PULL 1/2] hw/xen/xen_pvdev: Include qemu/log.h for qemu_log_vprintf()
From: Thomas Huth Olaf Hering reported a build failure due to an undefined reference to 'qemu_log_vprintf'. Explicitely including qemu/log.h seems to fix the issue. Signed-off-by: Thomas Huth Signed-off-by: Stefano Stabellini Acked-by: Stefano Stabellini Tested-by: Olaf Hering --- hw/xen/xen_pvdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c index 405e154..5212bc6 100644 --- a/hw/xen/xen_pvdev.c +++ b/hw/xen/xen_pvdev.c @@ -18,7 +18,7 @@ */ #include "qemu/osdep.h" - +#include "qemu/log.h" #include "hw/xen/xen_backend.h" #include "hw/xen/xen_pvdev.h" -- 1.9.1
Re: [Qemu-devel] [PATCH] hw/xen/xen_pvdev: Include qemu/log.h for qemu_log_vprintf()
On Wed, 2 Nov 2016, Peter Maydell wrote: > On 2 November 2016 at 17:34, Stefano Stabellini > wrote: > > On Wed, 2 Nov 2016, Thomas Huth wrote: > >> Olaf Hering reported a build failure due to an undefined reference > >> to 'qemu_log_vprintf'. Explicitely including qemu/log.h seems to > >> fix the issue. > >> > >> Signed-off-by: Thomas Huth > > > > The fix makes sense: > > > > Acked-by: Stefano Stabellini > > > > However I was unable to reproduce the build failure, so I would like a > > confirmation from Olaf that the fix is working. > > If you configure with the (default) simple trace > backend then trace.h will pull in qemu/log.h > (and in this case trace.h comes in via xen_common.h). > But if you're using a different backend then it won't > bring in that header, and you'll get the build failure. > > This is a fairly common "breaks but only with > some trace backends" compile failure, but I'm > not sure how best to try to make it more robust > (or at least into a 'fails-everywhere' bug). Personally what I can do is adding --enable-trace-backends=syslog to my test scripts. That is enough to repro the issue. Thanks for the explanation.
Re: [Qemu-devel] [PATCH] hw/xen/xen_pvdev: Include qemu/log.h for qemu_log_vprintf()
On 2 November 2016 at 17:34, Stefano Stabellini wrote: > On Wed, 2 Nov 2016, Thomas Huth wrote: >> Olaf Hering reported a build failure due to an undefined reference >> to 'qemu_log_vprintf'. Explicitely including qemu/log.h seems to >> fix the issue. >> >> Signed-off-by: Thomas Huth > > The fix makes sense: > > Acked-by: Stefano Stabellini > > However I was unable to reproduce the build failure, so I would like a > confirmation from Olaf that the fix is working. If you configure with the (default) simple trace backend then trace.h will pull in qemu/log.h (and in this case trace.h comes in via xen_common.h). But if you're using a different backend then it won't bring in that header, and you'll get the build failure. This is a fairly common "breaks but only with some trace backends" compile failure, but I'm not sure how best to try to make it more robust (or at least into a 'fails-everywhere' bug). thanks -- PMM
Re: [Qemu-devel] [PATCH v4 1/2] util/mmap-alloc: check parameter before using
On 02.11.2016 14:44, Cao jin wrote:
> Signed-off-by: Cao jin
> ---
> util/mmap-alloc.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/util/mmap-alloc.c b/util/mmap-alloc.c
> index 5a85aa3..d713a72 100644
> --- a/util/mmap-alloc.c
> +++ b/util/mmap-alloc.c
> @@ -12,6 +12,7 @@
>
> #include "qemu/osdep.h"
> #include "qemu/mmap-alloc.h"
> +#include "qemu/host-utils.h"
>
> #define HUGETLBFS_MAGIC 0x958458f6
>
> @@ -61,18 +62,18 @@ void *qemu_ram_mmap(int fd, size_t size, size_t align,
> bool shared)
> #else
> void *ptr = mmap(0, total, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1,
> 0);
> #endif
> -size_t offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
> +size_t offset;
> void *ptr1;
>
> if (ptr == MAP_FAILED) {
> return MAP_FAILED;
> }
>
> -/* Make sure align is a power of 2 */
> -assert(!(align & (align - 1)));
> +assert(is_power_of_2(align));
> /* Always align to host page size */
> assert(align >= getpagesize());
>
> +offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
> ptr1 = mmap(ptr + offset, size, PROT_READ | PROT_WRITE,
> MAP_FIXED |
> (fd == -1 ? MAP_ANONYMOUS : 0) |
>
Reviewed-by: Thomas Huth
Re: [Qemu-devel] [PATCH v2 0/3] atapi: classify read_cd as conditionally returning data
On 11/02/2016 03:49 PM, Hervé Poussineau wrote: Le 01/11/2016 à 04:16, John Snow a écrit : v2: - Actually applied the changes this time ... - And added a test to the AHCI suite... - ...Which revealed a few small issues in the suite. The AHCI test should be sufficient in terms of general proof for ATAPI regardless of the HBA used. All patches: Tested-by: Hervé Poussineau Great, thank you. Sorry it took so long for me to act on your original patch. For convenience, this branch is available at: https://github.com/jnsnow/qemu.git branch ide-fix-read-cd https://github.com/jnsnow/qemu/tree/ide-fix-read-cd This version is tagged ide-fix-read-cd-v2: https://github.com/jnsnow/qemu/releases/tag/ide-fix-read-cd-v2 John Snow (3): atapi: classify read_cd as conditionally returning data ahci-test: Create smaller test ISO images ahci-test: test atapi read_cd with bcl,nb_sectors = 0 hw/ide/atapi.c | 51 --- tests/ahci-test.c | 39 +++ tests/libqos/ahci.c | 27 --- tests/libqos/ahci.h | 17 ++--- 4 files changed, 101 insertions(+), 33 deletions(-)
[Qemu-devel] [PATCH] target-i386: fix typo
The impact is small because kvm_get_vcpu_events fixes env->hflags, but
it is wrong and could cause INITs to be delayed arbitrarily with
-machine kernel_irqchip=off.
Signed-off-by: Paolo Bonzini
---
target-i386/kvm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 1c0864e..f62264a 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2855,7 +2855,7 @@ MemTxAttrs kvm_arch_post_run(CPUState *cpu, struct
kvm_run *run)
if (run->flags & KVM_RUN_X86_SMM) {
env->hflags |= HF_SMM_MASK;
} else {
-env->hflags &= HF_SMM_MASK;
+env->hflags &= ~HF_SMM_MASK;
}
if (run->if_flag) {
env->eflags |= IF_MASK;
--
2.7.4
Re: [Qemu-devel] [PATCH v2 0/3] atapi: classify read_cd as conditionally returning data
Le 01/11/2016 à 04:16, John Snow a écrit : v2: - Actually applied the changes this time ... - And added a test to the AHCI suite... - ...Which revealed a few small issues in the suite. The AHCI test should be sufficient in terms of general proof for ATAPI regardless of the HBA used. All patches: Tested-by: Hervé Poussineau For convenience, this branch is available at: https://github.com/jnsnow/qemu.git branch ide-fix-read-cd https://github.com/jnsnow/qemu/tree/ide-fix-read-cd This version is tagged ide-fix-read-cd-v2: https://github.com/jnsnow/qemu/releases/tag/ide-fix-read-cd-v2 John Snow (3): atapi: classify read_cd as conditionally returning data ahci-test: Create smaller test ISO images ahci-test: test atapi read_cd with bcl,nb_sectors = 0 hw/ide/atapi.c | 51 --- tests/ahci-test.c | 39 +++ tests/libqos/ahci.c | 27 --- tests/libqos/ahci.h | 17 ++--- 4 files changed, 101 insertions(+), 33 deletions(-)
Re: [Qemu-devel] [PATCH] hw/xen/xen_pvdev: Include qemu/log.h for qemu_log_vprintf()
On Wed, Nov 02, Thomas Huth wrote: > Olaf Hering reported a build failure due to an undefined reference > to 'qemu_log_vprintf'. Explicitely including qemu/log.h seems to > fix the issue. > > Signed-off-by: Thomas Huth Tested-by: Olaf Hering Olaf signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH v2 0/3] atapi: classify read_cd as conditionally returning data
On 10/31/2016 11:16 PM, John Snow wrote: v2: - Actually applied the changes this time ... - And added a test to the AHCI suite... - ...Which revealed a few small issues in the suite. The AHCI test should be sufficient in terms of general proof for ATAPI regardless of the HBA used. For convenience, this branch is available at: https://github.com/jnsnow/qemu.git branch ide-fix-read-cd https://github.com/jnsnow/qemu/tree/ide-fix-read-cd This version is tagged ide-fix-read-cd-v2: https://github.com/jnsnow/qemu/releases/tag/ide-fix-read-cd-v2 John Snow (3): atapi: classify read_cd as conditionally returning data ahci-test: Create smaller test ISO images ahci-test: test atapi read_cd with bcl,nb_sectors = 0 hw/ide/atapi.c | 51 --- tests/ahci-test.c | 39 +++ tests/libqos/ahci.c | 27 --- tests/libqos/ahci.h | 17 ++--- 4 files changed, 101 insertions(+), 33 deletions(-) Thanks, applied to my IDE tree: https://github.com/jnsnow/qemu/commits/ide https://github.com/jnsnow/qemu.git --js
Re: [Qemu-devel] [PATCH for-2.8? 3/3] block/curl: Drop TFTP "support"
On Wed, Nov 02, 2016 at 06:55:39PM +0100, Max Reitz wrote:
> Because TFTP does not support byte ranges, it was never usable with our
> curl block driver. Since apparently nobody has ever complained loudly
> enough for someone to take care of the issue until now, it seems
> reasonable to assume that nobody has ever actually used it.
>
> Therefore, it should be safe to just drop it from curl's protocol list.
>
> Signed-off-by: Max Reitz
> ---
> block/curl.c | 20 +---
> 1 file changed, 1 insertion(+), 19 deletions(-)
>
> diff --git a/block/curl.c b/block/curl.c
> index e5eaa7b..ba8adae 100644
> --- a/block/curl.c
> +++ b/block/curl.c
> @@ -68,8 +68,7 @@ static CURLMcode __curl_multi_socket_action(CURLM
> *multi_handle,
> #endif
>
> #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \
> - CURLPROTO_FTP | CURLPROTO_FTPS | \
> - CURLPROTO_TFTP)
> + CURLPROTO_FTP | CURLPROTO_FTPS)
>
> #define CURL_NUM_STATES 8
> #define CURL_NUM_ACB8
> @@ -886,29 +885,12 @@ static BlockDriver bdrv_ftps = {
> .bdrv_attach_aio_context= curl_attach_aio_context,
> };
>
> -static BlockDriver bdrv_tftp = {
> -.format_name= "tftp",
> -.protocol_name = "tftp",
> -
> -.instance_size = sizeof(BDRVCURLState),
> -.bdrv_parse_filename= curl_parse_filename,
> -.bdrv_file_open = curl_open,
> -.bdrv_close = curl_close,
> -.bdrv_getlength = curl_getlength,
> -
> -.bdrv_aio_readv = curl_aio_readv,
> -
> -.bdrv_detach_aio_context= curl_detach_aio_context,
> -.bdrv_attach_aio_context= curl_attach_aio_context,
> -};
> -
> static void curl_block_init(void)
> {
> bdrv_register(&bdrv_http);
> bdrv_register(&bdrv_https);
> bdrv_register(&bdrv_ftp);
> bdrv_register(&bdrv_ftps);
> -bdrv_register(&bdrv_tftp);
> }
>
> block_init(curl_block_init);
> --
> 2.10.2
>
Reviewed-by: Jeff Cody
Re: [Qemu-devel] [PATCH for-2.8? 2/3] qapi: Drop curl's TFTP protocol
On Wed, Nov 02, 2016 at 06:55:38PM +0100, Max Reitz wrote: > A follow-up patch will remove the curl block driver's TFTP support, so > remove the protocol from the QAPI schema. > > Signed-off-by: Max Reitz > --- > docs/qmp-commands.txt | 2 +- > qapi/block-core.json | 7 +++ > 2 files changed, 4 insertions(+), 5 deletions(-) > > diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt > index 6afa872..abf210a 100644 > --- a/docs/qmp-commands.txt > +++ b/docs/qmp-commands.txt > @@ -1803,7 +1803,7 @@ Each json-object contain the following: > "file", "file", "ftp", "ftps", "host_cdrom", > "host_device", "http", "https", > "nbd", "parallels", "qcow", "qcow2", "raw", > -"tftp", "vdi", "vmdk", "vpc", "vvfat" > +"vdi", "vmdk", "vpc", "vvfat" > - "backing_file": backing file name (json-string, optional) > - "backing_file_depth": number of files in the backing file chain > (json-int) > - "encrypted": true if encrypted, false otherwise (json-bool) > diff --git a/qapi/block-core.json b/qapi/block-core.json > index bcd3b9e..c29bef7 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -243,12 +243,12 @@ > # 0.14.0 this can be: 'blkdebug', 'bochs', 'cloop', 'cow', 'dmg', > # 'file', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device', > # 'http', 'https', 'luks', 'nbd', 'parallels', 'qcow', > -# 'qcow2', 'raw', 'tftp', 'vdi', 'vmdk', 'vpc', 'vvfat' > +# 'qcow2', 'raw', 'vdi', 'vmdk', 'vpc', 'vvfat' > # 2.2: 'archipelago' added, 'cow' dropped > # 2.3: 'host_floppy' deprecated > # 2.5: 'host_floppy' dropped > # 2.6: 'luks' added > -# 2.8: 'replication' added > +# 2.8: 'replication' added, 'tftp' dropped > # > # @backing_file: #optional the name of the backing file (for copy-on-write) > # > @@ -1723,7 +1723,7 @@ > 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom', > 'host_device', 'http', 'https', 'luks', 'nbd', 'nfs', 'null-aio', > 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', > -'replication', 'ssh', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', > +'replication', 'ssh', 'vdi', 'vhdx', 'vmdk', 'vpc', > 'vvfat' ] } > > ## > @@ -2410,7 +2410,6 @@ >'replication':'BlockdevOptionsReplication', > # TODO sheepdog: Wait for structured options >'ssh':'BlockdevOptionsSsh', > - 'tftp': 'BlockdevOptionsCurl', >'vdi':'BlockdevOptionsGenericFormat', >'vhdx': 'BlockdevOptionsGenericFormat', >'vmdk': 'BlockdevOptionsGenericCOWFormat', > -- > 2.10.2 > Reviewed-by: Jeff Cody
Re: [Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support"
On Wed, Nov 02, 2016 at 06:55:36PM +0100, Max Reitz wrote: > See patch 3 for the reason why we have actually never supported TFTP at > all (except for very small files (i.e. below 256 kB or so)). > > I would consider this series a bug fix because, well, it doesn't really > change any functionality, and the bug is "We don't support TFTP but we > pretend we do". > I tend to agree. I'm willing to pull this in through my branch for 2.8, unless there arises some outcry with good reason to keep tftp. > > Alternatives to this approach: > > - Deprecate TFTP first. Wait one version, then drop it. > > We could do this, but I personally don't think it's necessary. We have > done this for host_floppy, but in contrast to host_floppy, TFTP really > has never worked. Thus, I conclude that nobody is actually using it or > has ever used it for real work. > > Still, if you think otherwise, we can still do this, of course. > > > - Don't remove TFTP altogether, but just emit a run-time error like we > do for HTTP servers that do not support range-based requests. > > Seems dirty and not like the real solution to me. Also, we have > removed other block drivers in the past, so I don't think we should > keep TFTP. > Since it is broken by nature, I like your original approach of just removing it. > > Max Reitz (3): > qemu-options: Drop mentions of curl's TFTP support > qapi: Drop curl's TFTP protocol > block/curl: Drop TFTP "support" > > block/curl.c | 20 +--- > docs/qmp-commands.txt | 2 +- > qapi/block-core.json | 7 +++ > qemu-options.hx | 6 +++--- > 4 files changed, 8 insertions(+), 27 deletions(-) A > 3:1 delete to insert ratio, that is an ideal patch series ;-)
Re: [Qemu-devel] [PATCH for-2.8? 1/3] qemu-options: Drop mentions of curl's TFTP support
On Wed, Nov 02, 2016 at 06:55:37PM +0100, Max Reitz wrote:
> A follow-up patch will remove the curl block driver's TFTP support;
> therefore, we should no longer mention it anywhere in our documentation.
>
> Signed-off-by: Max Reitz
> ---
> qemu-options.hx | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 95332cc..7212779 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -2606,8 +2606,8 @@ qemu-system-x86_64 --drive
> file=gluster://192.0.2.1/testvol/a.img
>
> See also @url{http://www.gluster.org}.
>
> -@item HTTP/HTTPS/FTP/FTPS/TFTP
> -QEMU supports read-only access to files accessed over http(s), ftp(s) and
> tftp.
> +@item HTTP/HTTPS/FTP/FTPS
> +QEMU supports read-only access to files accessed over http(s) and ftp(s).
>
> Syntax using a single filename:
> @example
> @@ -2617,7 +2617,7 @@ Syntax using a single filename:
> where:
> @table @option
> @item protocol
> -'http', 'https', 'ftp', 'ftps', or 'tftp'.
> +'http', 'https', 'ftp', or 'ftps'.
>
> @item username
> Optional username for authentication to the remote server.
> --
> 2.10.2
>
Reviewed-by: Jeff Cody
Re: [Qemu-devel] [PATCH v2] mttcg: Handle EXCP_ATOMIC exception
Paolo Bonzini writes:
> On 02/11/2016 17:40, Pranith Kumar wrote:
>> The patch enables handling atomic code in the guest. This should be
>> preferably done in cpu_handle_exception(), but the current assumptions
>> regarding when we can execute atomic sections cause a deadlock.
>>
>> Signed-off-by: Pranith Kumar
>> ---
>> cpus.c | 9 +
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 8f98060..299ce7e 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -1315,6 +1315,11 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
>> if (r == EXCP_DEBUG) {
>> cpu_handle_guest_debug(cpu);
>> break;
>> +} else if (r == EXCP_ATOMIC) {
>> +qemu_mutex_unlock_iothread();
>> +cpu_exec_step_atomic(cpu);
>> +qemu_mutex_lock_iothread();
>> +break;
>> }
>> } else if (cpu->stop) {
>> if (cpu->unplug) {
>> @@ -1385,6 +1390,10 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
>> */
>> g_assert(cpu->halted);
>> break;
>> +case EXCP_ATOMIC:
>> +qemu_mutex_unlock_iothread();
>> +cpu_exec_step_atomic(cpu);
>> +qemu_mutex_lock_iothread();
>> default:
>> /* Ignore everything else? */
>> break;
>>
>
> Alex, please pick up this patch yourself.
Yep, I'll apply it to my tree.
>
> Paolo
--
Alex Bennée
Re: [Qemu-devel] [Qemu-block] [PULL v2 00/14] Block patches for 2.8
On 2 November 2016 at 17:03, Stefan Hajnoczi wrote: > On Tue, Nov 01, 2016 at 08:50:57AM -0400, Jeff Cody wrote: >> The following changes since commit bf99fd3983d7185178a0f65ce29bb94b1aecaed1: >> >> Merge remote-tracking branch 'remotes/rth/tags/pull-sparc-20161031-2' into >> staging (2016-11-01 11:21:02 +) >> >> are available in the git repository at: >> >> g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request > > This pull request does not have a publicly-accessible URI. > > Please adjust your .gitconfig: > > [remote "public"] > url = git://github.com/codyprime/qemu.git > pushurl = g...@github.com:codyprime/qemu.git I think the apply-pullreq script can cope with that, once you've added the remote to your git repo -- there's slack in the "figure out the remote given the URL" logic to cope with github having a bunch of different ways to represent the same repo and maintainers tending to flip between them. thanks -- PMM
[Qemu-devel] [PATCH for-2.8? 3/3] block/curl: Drop TFTP "support"
Because TFTP does not support byte ranges, it was never usable with our
curl block driver. Since apparently nobody has ever complained loudly
enough for someone to take care of the issue until now, it seems
reasonable to assume that nobody has ever actually used it.
Therefore, it should be safe to just drop it from curl's protocol list.
Signed-off-by: Max Reitz
---
block/curl.c | 20 +---
1 file changed, 1 insertion(+), 19 deletions(-)
diff --git a/block/curl.c b/block/curl.c
index e5eaa7b..ba8adae 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -68,8 +68,7 @@ static CURLMcode __curl_multi_socket_action(CURLM
*multi_handle,
#endif
#define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \
- CURLPROTO_FTP | CURLPROTO_FTPS | \
- CURLPROTO_TFTP)
+ CURLPROTO_FTP | CURLPROTO_FTPS)
#define CURL_NUM_STATES 8
#define CURL_NUM_ACB8
@@ -886,29 +885,12 @@ static BlockDriver bdrv_ftps = {
.bdrv_attach_aio_context= curl_attach_aio_context,
};
-static BlockDriver bdrv_tftp = {
-.format_name= "tftp",
-.protocol_name = "tftp",
-
-.instance_size = sizeof(BDRVCURLState),
-.bdrv_parse_filename= curl_parse_filename,
-.bdrv_file_open = curl_open,
-.bdrv_close = curl_close,
-.bdrv_getlength = curl_getlength,
-
-.bdrv_aio_readv = curl_aio_readv,
-
-.bdrv_detach_aio_context= curl_detach_aio_context,
-.bdrv_attach_aio_context= curl_attach_aio_context,
-};
-
static void curl_block_init(void)
{
bdrv_register(&bdrv_http);
bdrv_register(&bdrv_https);
bdrv_register(&bdrv_ftp);
bdrv_register(&bdrv_ftps);
-bdrv_register(&bdrv_tftp);
}
block_init(curl_block_init);
--
2.10.2
[Qemu-devel] [PATCH for-2.8? 1/3] qemu-options: Drop mentions of curl's TFTP support
A follow-up patch will remove the curl block driver's TFTP support;
therefore, we should no longer mention it anywhere in our documentation.
Signed-off-by: Max Reitz
---
qemu-options.hx | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/qemu-options.hx b/qemu-options.hx
index 95332cc..7212779 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2606,8 +2606,8 @@ qemu-system-x86_64 --drive
file=gluster://192.0.2.1/testvol/a.img
See also @url{http://www.gluster.org}.
-@item HTTP/HTTPS/FTP/FTPS/TFTP
-QEMU supports read-only access to files accessed over http(s), ftp(s) and tftp.
+@item HTTP/HTTPS/FTP/FTPS
+QEMU supports read-only access to files accessed over http(s) and ftp(s).
Syntax using a single filename:
@example
@@ -2617,7 +2617,7 @@ Syntax using a single filename:
where:
@table @option
@item protocol
-'http', 'https', 'ftp', 'ftps', or 'tftp'.
+'http', 'https', 'ftp', or 'ftps'.
@item username
Optional username for authentication to the remote server.
--
2.10.2
[Qemu-devel] [PATCH for-2.8? 2/3] qapi: Drop curl's TFTP protocol
A follow-up patch will remove the curl block driver's TFTP support, so remove the protocol from the QAPI schema. Signed-off-by: Max Reitz --- docs/qmp-commands.txt | 2 +- qapi/block-core.json | 7 +++ 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/qmp-commands.txt b/docs/qmp-commands.txt index 6afa872..abf210a 100644 --- a/docs/qmp-commands.txt +++ b/docs/qmp-commands.txt @@ -1803,7 +1803,7 @@ Each json-object contain the following: "file", "file", "ftp", "ftps", "host_cdrom", "host_device", "http", "https", "nbd", "parallels", "qcow", "qcow2", "raw", -"tftp", "vdi", "vmdk", "vpc", "vvfat" +"vdi", "vmdk", "vpc", "vvfat" - "backing_file": backing file name (json-string, optional) - "backing_file_depth": number of files in the backing file chain (json-int) - "encrypted": true if encrypted, false otherwise (json-bool) diff --git a/qapi/block-core.json b/qapi/block-core.json index bcd3b9e..c29bef7 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -243,12 +243,12 @@ # 0.14.0 this can be: 'blkdebug', 'bochs', 'cloop', 'cow', 'dmg', # 'file', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device', # 'http', 'https', 'luks', 'nbd', 'parallels', 'qcow', -# 'qcow2', 'raw', 'tftp', 'vdi', 'vmdk', 'vpc', 'vvfat' +# 'qcow2', 'raw', 'vdi', 'vmdk', 'vpc', 'vvfat' # 2.2: 'archipelago' added, 'cow' dropped # 2.3: 'host_floppy' deprecated # 2.5: 'host_floppy' dropped # 2.6: 'luks' added -# 2.8: 'replication' added +# 2.8: 'replication' added, 'tftp' dropped # # @backing_file: #optional the name of the backing file (for copy-on-write) # @@ -1723,7 +1723,7 @@ 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom', 'host_device', 'http', 'https', 'luks', 'nbd', 'nfs', 'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', -'replication', 'ssh', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', +'replication', 'ssh', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] } ## @@ -2410,7 +2410,6 @@ 'replication':'BlockdevOptionsReplication', # TODO sheepdog: Wait for structured options 'ssh':'BlockdevOptionsSsh', - 'tftp': 'BlockdevOptionsCurl', 'vdi':'BlockdevOptionsGenericFormat', 'vhdx': 'BlockdevOptionsGenericFormat', 'vmdk': 'BlockdevOptionsGenericCOWFormat', -- 2.10.2
[Qemu-devel] [PATCH for-2.8? 0/3] block/curl: Drop TFTP "support"
See patch 3 for the reason why we have actually never supported TFTP at all (except for very small files (i.e. below 256 kB or so)). I would consider this series a bug fix because, well, it doesn't really change any functionality, and the bug is "We don't support TFTP but we pretend we do". Alternatives to this approach: - Deprecate TFTP first. Wait one version, then drop it. We could do this, but I personally don't think it's necessary. We have done this for host_floppy, but in contrast to host_floppy, TFTP really has never worked. Thus, I conclude that nobody is actually using it or has ever used it for real work. Still, if you think otherwise, we can still do this, of course. - Don't remove TFTP altogether, but just emit a run-time error like we do for HTTP servers that do not support range-based requests. Seems dirty and not like the real solution to me. Also, we have removed other block drivers in the past, so I don't think we should keep TFTP. Max Reitz (3): qemu-options: Drop mentions of curl's TFTP support qapi: Drop curl's TFTP protocol block/curl: Drop TFTP "support" block/curl.c | 20 +--- docs/qmp-commands.txt | 2 +- qapi/block-core.json | 7 +++ qemu-options.hx | 6 +++--- 4 files changed, 8 insertions(+), 27 deletions(-) -- 2.10.2
[Qemu-devel] [PATCH v3 4/6] blockjob: add block_job_start
Instead of automatically starting jobs at creation time via backup_start
et al, we'd like to return a job object pointer that can be started
manually at later point in time.
For now, add the block_job_start mechanism and start the jobs
automatically as we have been doing, with conversions job-by-job coming
in later patches.
Of note: cancellation of unstarted jobs will perform all the normal
cleanup as if the job had started, particularly abort and clean. The
only difference is that we will not emit any events, because the job
never actually started.
Signed-off-by: John Snow
---
block/backup.c| 3 +--
block/commit.c| 3 +--
block/mirror.c| 3 +--
block/stream.c| 3 +--
blockjob.c| 51 ---
include/block/blockjob.h | 9 +
tests/test-blockjob-txn.c | 12 +--
7 files changed, 58 insertions(+), 26 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 4ed4494..ae1b99a 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -654,9 +654,8 @@ void backup_start(const char *job_id, BlockDriverState *bs,
block_job_add_bdrv(&job->common, target);
job->common.len = len;
-job->common.co = qemu_coroutine_create(job->common.driver->start, job);
block_job_txn_add_job(txn, &job->common);
-qemu_coroutine_enter(job->common.co);
+block_job_start(&job->common);
return;
error:
diff --git a/block/commit.c b/block/commit.c
index 20d27e2..5b7c454 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -289,10 +289,9 @@ void commit_start(const char *job_id, BlockDriverState *bs,
s->backing_file_str = g_strdup(backing_file_str);
s->on_error = on_error;
-s->common.co = qemu_coroutine_create(s->common.driver->start, s);
trace_commit_start(bs, base, top, s, s->common.co);
-qemu_coroutine_enter(s->common.co);
+block_job_start(&s->common);
}
diff --git a/block/mirror.c b/block/mirror.c
index 659e09c..c078d45 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1009,9 +1009,8 @@ static void mirror_start_job(const char *job_id,
BlockDriverState *bs,
}
}
-s->common.co = qemu_coroutine_create(s->common.driver->start, s);
trace_mirror_start(bs, s, s->common.co, opaque);
-qemu_coroutine_enter(s->common.co);
+block_job_start(&s->common);
}
void mirror_start(const char *job_id, BlockDriverState *bs,
diff --git a/block/stream.c b/block/stream.c
index 92309ff..2de8d38 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -255,7 +255,6 @@ void stream_start(const char *job_id, BlockDriverState *bs,
s->bs_flags = orig_bs_flags;
s->on_error = on_error;
-s->common.co = qemu_coroutine_create(s->common.driver->start, s);
trace_stream_start(bs, base, s, s->common.co);
-qemu_coroutine_enter(s->common.co);
+block_job_start(&s->common);
}
diff --git a/blockjob.c b/blockjob.c
index e3c458c..16c5159 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -174,7 +174,8 @@ void *block_job_create(const char *job_id, const
BlockJobDriver *driver,
job->blk = blk;
job->cb= cb;
job->opaque= opaque;
-job->busy = true;
+job->busy = false;
+job->paused= true;
job->refcnt= 1;
bs->job = job;
@@ -202,6 +203,21 @@ bool block_job_is_internal(BlockJob *job)
return (job->id == NULL);
}
+static bool block_job_started(BlockJob *job)
+{
+return job->co;
+}
+
+void block_job_start(BlockJob *job)
+{
+assert(job && !block_job_started(job) && job->paused &&
+ !job->busy && job->driver->start);
+job->paused = false;
+job->busy = true;
+job->co = qemu_coroutine_create(job->driver->start, job);
+qemu_coroutine_enter(job->co);
+}
+
void block_job_ref(BlockJob *job)
{
++job->refcnt;
@@ -248,14 +264,18 @@ static void block_job_completed_single(BlockJob *job)
if (job->cb) {
job->cb(job->opaque, job->ret);
}
-if (block_job_is_cancelled(job)) {
-block_job_event_cancelled(job);
-} else {
-const char *msg = NULL;
-if (job->ret < 0) {
-msg = strerror(-job->ret);
+
+/* Emit events only if we actually started */
+if (block_job_started(job)) {
+if (block_job_is_cancelled(job)) {
+block_job_event_cancelled(job);
+} else {
+const char *msg = NULL;
+if (job->ret < 0) {
+msg = strerror(-job->ret);
+}
+block_job_event_completed(job, msg);
}
-block_job_event_completed(job, msg);
}
if (job->txn) {
@@ -363,7 +383,8 @@ void block_job_complete(BlockJob *job, Error **errp)
{
/* Should not be reachable via external interface for internal jobs */
assert(job->id);
-if (job->pause_count || job->cancelled || !job->driver->complete) {
+if (job->pause_count || job->cancelled ||
+!block_job_sta
[Qemu-devel] [PATCH v3 5/6] blockjob: refactor backup_start as backup_job_create
Refactor backup_start as backup_job_create, which only creates the job,
but does not automatically start it. The old interface, 'backup_start',
is not kept in favor of limiting the number of nearly-identical interfaces
that would have to be edited to keep up with QAPI changes in the future.
Callers that wish to synchronously start the backup_block_job can
instead just call block_job_start immediately after calling
backup_job_create.
Transactions are updated to use the new interface, calling block_job_start
only during the .commit phase, which helps prevent race conditions where
jobs may finish before we even finish building the transaction. This may
happen, for instance, during empty block backup jobs.
Reported-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: John Snow
---
block/backup.c| 26 ---
block/replication.c | 12 ---
blockdev.c| 83 ++-
include/block/block_int.h | 23 ++---
4 files changed, 87 insertions(+), 57 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index ae1b99a..ea38733 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -543,7 +543,7 @@ static const BlockJobDriver backup_job_driver = {
.drain = backup_drain,
};
-void backup_start(const char *job_id, BlockDriverState *bs,
+BlockJob *backup_job_create(const char *job_id, BlockDriverState *bs,
BlockDriverState *target, int64_t speed,
MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
bool compress,
@@ -563,52 +563,52 @@ void backup_start(const char *job_id, BlockDriverState
*bs,
if (bs == target) {
error_setg(errp, "Source and target cannot be the same");
-return;
+return NULL;
}
if (!bdrv_is_inserted(bs)) {
error_setg(errp, "Device is not inserted: %s",
bdrv_get_device_name(bs));
-return;
+return NULL;
}
if (!bdrv_is_inserted(target)) {
error_setg(errp, "Device is not inserted: %s",
bdrv_get_device_name(target));
-return;
+return NULL;
}
if (compress && target->drv->bdrv_co_pwritev_compressed == NULL) {
error_setg(errp, "Compression is not supported for this drive %s",
bdrv_get_device_name(target));
-return;
+return NULL;
}
if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
-return;
+return NULL;
}
if (bdrv_op_is_blocked(target, BLOCK_OP_TYPE_BACKUP_TARGET, errp)) {
-return;
+return NULL;
}
if (sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
if (!sync_bitmap) {
error_setg(errp, "must provide a valid bitmap name for "
"\"incremental\" sync mode");
-return;
+return NULL;
}
/* Create a new bitmap, and freeze/disable this one. */
if (bdrv_dirty_bitmap_create_successor(bs, sync_bitmap, errp) < 0) {
-return;
+return NULL;
}
} else if (sync_bitmap) {
error_setg(errp,
"a sync_bitmap was provided to backup_run, "
"but received an incompatible sync_mode (%s)",
MirrorSyncMode_lookup[sync_mode]);
-return;
+return NULL;
}
len = bdrv_getlength(bs);
@@ -655,8 +655,8 @@ void backup_start(const char *job_id, BlockDriverState *bs,
block_job_add_bdrv(&job->common, target);
job->common.len = len;
block_job_txn_add_job(txn, &job->common);
-block_job_start(&job->common);
-return;
+
+return &job->common;
error:
if (sync_bitmap) {
@@ -666,4 +666,6 @@ void backup_start(const char *job_id, BlockDriverState *bs,
backup_clean(&job->common);
block_job_unref(&job->common);
}
+
+return NULL;
}
diff --git a/block/replication.c b/block/replication.c
index d5e2b0f..729dd12 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -421,6 +421,7 @@ static void replication_start(ReplicationState *rs,
ReplicationMode mode,
int64_t active_length, hidden_length, disk_length;
AioContext *aio_context;
Error *local_err = NULL;
+BlockJob *job;
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
@@ -508,17 +509,18 @@ static void replication_start(ReplicationState *rs,
ReplicationMode mode,
bdrv_op_block_all(top_bs, s->blocker);
bdrv_op_unblock(top_bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker);
-backup_start(NULL, s->secondary_disk->bs, s->hidden_disk->bs, 0,
- MIRROR_SYNC_MODE_NONE, NULL, false,
- BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
- BLOCK_JOB_INTERNAL, backup_job_completed, bs,
- NULL, &local_err);
+job = backup_job_crea
[Qemu-devel] [PATCH v3 0/6] jobs: fix transactional race condition
There are a few problems with transactional job completion right now. First, if jobs complete so quickly they complete before remaining jobs get a chance to join the transaction, the completion mode can leave well known state and the QLIST can get corrupted and the transactional jobs can complete in batches or phases instead of all together. Second, if two or more jobs defer to the main loop at roughly the same time, it's possible for one job's cleanup to directly invoke the other job's cleanup from within the same thread, leading to a situation that will deadlock the entire transaction. Thanks to Vladimir for pointing out these modes of failure. === v3: === - Rebase to origin/master, requisite patches now upstream. === v2: === - Correct Vladimir's email (Sorry!) - Add test as a variant of an existing test [Vladimir] For convenience, this branch is available at: https://github.com/jnsnow/qemu.git branch job-fix-race-condition https://github.com/jnsnow/qemu/tree/job-fix-race-condition This version is tagged job-fix-race-condition-v3: https://github.com/jnsnow/qemu/releases/tag/job-fix-race-condition-v3 John Snow (5): blockjob: add .clean property blockjob: add .start field blockjob: add block_job_start blockjob: refactor backup_start as backup_job_create iotests: add transactional failure race test Vladimir Sementsov-Ogievskiy (1): blockjob: fix dead pointer in txn list block/backup.c | 63 ++--- block/commit.c | 4 +-- block/mirror.c | 5 +-- block/replication.c | 12 --- block/stream.c | 4 +-- blockdev.c | 83 blockjob.c | 55 ++--- include/block/block_int.h| 23 ++-- include/block/blockjob.h | 9 + include/block/blockjob_int.h | 11 ++ tests/qemu-iotests/124 | 53 ++-- tests/qemu-iotests/124.out | 4 +-- tests/test-blockjob-txn.c| 12 +++ 13 files changed, 221 insertions(+), 117 deletions(-) -- 2.7.4
[Qemu-devel] [PATCH v3 6/6] iotests: add transactional failure race test
Add a regression test for the case found by Vladimir.
Reported-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: John Snow
---
tests/qemu-iotests/124 | 53 ++
tests/qemu-iotests/124.out | 4 ++--
2 files changed, 37 insertions(+), 20 deletions(-)
diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
index f06938e..d0d2c2b 100644
--- a/tests/qemu-iotests/124
+++ b/tests/qemu-iotests/124
@@ -395,19 +395,7 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
self.check_backups()
-def test_transaction_failure(self):
-'''Test: Verify backups made from a transaction that partially fails.
-
-Add a second drive with its own unique pattern, and add a bitmap to
each
-drive. Use blkdebug to interfere with the backup on just one drive and
-attempt to create a coherent incremental backup across both drives.
-
-verify a failure in one but not both, then delete the failed stubs and
-re-run the same transaction.
-
-verify that both incrementals are created successfully.
-'''
-
+def do_transaction_failure_test(self, race=False):
# Create a second drive, with pattern:
drive1 = self.add_node('drive1')
self.img_create(drive1['file'], drive1['fmt'])
@@ -451,9 +439,10 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
self.assertFalse(self.vm.get_qmp_events(wait=False))
# Emulate some writes
-self.hmp_io_writes(drive0['id'], (('0xab', 0, 512),
- ('0xfe', '16M', '256k'),
- ('0x64', '32736k', '64k')))
+if not race:
+self.hmp_io_writes(drive0['id'], (('0xab', 0, 512),
+ ('0xfe', '16M', '256k'),
+ ('0x64', '32736k', '64k')))
self.hmp_io_writes(drive1['id'], (('0xba', 0, 512),
('0xef', '16M', '256k'),
('0x46', '32736k', '64k')))
@@ -463,7 +452,8 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
target1 = self.prepare_backup(dr1bm0)
# Ask for a new incremental backup per-each drive,
-# expecting drive1's backup to fail:
+# expecting drive1's backup to fail. In the 'race' test,
+# we expect drive1 to attempt to cancel the empty drive0 job.
transaction = [
transaction_drive_backup(drive0['id'], target0, sync='incremental',
format=drive0['fmt'], mode='existing',
@@ -488,9 +478,15 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
self.assert_no_active_block_jobs()
# Delete drive0's successful target and eliminate our record of the
-# unsuccessful drive1 target. Then re-run the same transaction.
+# unsuccessful drive1 target.
dr0bm0.del_target()
dr1bm0.del_target()
+if race:
+# Don't re-run the transaction, we only wanted to test the race.
+self.vm.shutdown()
+return
+
+# Re-run the same transaction:
target0 = self.prepare_backup(dr0bm0)
target1 = self.prepare_backup(dr1bm0)
@@ -511,6 +507,27 @@ class TestIncrementalBackup(TestIncrementalBackupBase):
self.vm.shutdown()
self.check_backups()
+def test_transaction_failure(self):
+'''Test: Verify backups made from a transaction that partially fails.
+
+Add a second drive with its own unique pattern, and add a bitmap to
each
+drive. Use blkdebug to interfere with the backup on just one drive and
+attempt to create a coherent incremental backup across both drives.
+
+verify a failure in one but not both, then delete the failed stubs and
+re-run the same transaction.
+
+verify that both incrementals are created successfully.
+'''
+self.do_transaction_failure_test()
+
+def test_transaction_failure_race(self):
+'''Test: Verify that transactions with jobs that have no data to
+transfer do not cause race conditions in the cancellation of the entire
+transaction job group.
+'''
+self.do_transaction_failure_test(race=True)
+
def test_sync_dirty_bitmap_missing(self):
self.assert_no_active_block_jobs()
diff --git a/tests/qemu-iotests/124.out b/tests/qemu-iotests/124.out
index 36376be..e56cae0 100644
--- a/tests/qemu-iotests/124.out
+++ b/tests/qemu-iotests/124.out
@@ -1,5 +1,5 @@
-..
+...
--
-Ran 10 tests
+Ran 11 tests
OK
--
2.7.4
[Qemu-devel] [PATCH v3 2/6] blockjob: add .clean property
Cleaning up after we have deferred to the main thread but before the
transaction has converged can be dangerous and result in deadlocks
if the job cleanup invokes any BH polling loops.
A job may attempt to begin cleaning up, but may induce another job to
enter its cleanup routine. The second job, part of our same transaction,
will block waiting for the first job to finish, so neither job may now
make progress.
To rectify this, allow jobs to register a cleanup operation that will
always run regardless of if the job was in a transaction or not, and
if the transaction job group completed successfully or not.
Move sensitive cleanup to this callback instead which is guaranteed to
be run only after the transaction has converged, which removes sensitive
timing constraints from said cleanup.
Furthermore, in future patches these cleanup operations will be performed
regardless of whether or not we actually started the job. Therefore,
cleanup callbacks should essentially confine themselves to undoing create
operations, e.g. setup actions taken in what is now backup_start.
Reported-by: Vladimir Sementsov-Ogievskiy
Signed-off-by: John Snow
---
block/backup.c | 15 ++-
blockjob.c | 3 +++
include/block/blockjob_int.h | 8
3 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 7b5d8a3..734a24c 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -242,6 +242,14 @@ static void backup_abort(BlockJob *job)
}
}
+static void backup_clean(BlockJob *job)
+{
+BackupBlockJob *s = container_of(job, BackupBlockJob, common);
+assert(s->target);
+blk_unref(s->target);
+s->target = NULL;
+}
+
static void backup_attached_aio_context(BlockJob *job, AioContext *aio_context)
{
BackupBlockJob *s = container_of(job, BackupBlockJob, common);
@@ -321,6 +329,7 @@ static const BlockJobDriver backup_job_driver = {
.set_speed = backup_set_speed,
.commit = backup_commit,
.abort = backup_abort,
+.clean = backup_clean,
.attached_aio_context = backup_attached_aio_context,
.drain = backup_drain,
};
@@ -343,12 +352,8 @@ typedef struct {
static void backup_complete(BlockJob *job, void *opaque)
{
-BackupBlockJob *s = container_of(job, BackupBlockJob, common);
BackupCompleteData *data = opaque;
-blk_unref(s->target);
-s->target = NULL;
-
block_job_completed(job, data->ret);
g_free(data);
}
@@ -658,7 +663,7 @@ void backup_start(const char *job_id, BlockDriverState *bs,
bdrv_reclaim_dirty_bitmap(bs, sync_bitmap, NULL);
}
if (job) {
-blk_unref(job->target);
+backup_clean(&job->common);
block_job_unref(&job->common);
}
}
diff --git a/blockjob.c b/blockjob.c
index 4d0ef53..e3c458c 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -241,6 +241,9 @@ static void block_job_completed_single(BlockJob *job)
job->driver->abort(job);
}
}
+if (job->driver->clean) {
+job->driver->clean(job);
+}
if (job->cb) {
job->cb(job->opaque, job->ret);
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 40275e4..60d91a0 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -74,6 +74,14 @@ struct BlockJobDriver {
void (*abort)(BlockJob *job);
/**
+ * If the callback is not NULL, it will be invoked after a call to either
+ * .commit() or .abort(). Regardless of which callback is invoked after
+ * completion, .clean() will always be called, even if the job does not
+ * belong to a transaction group.
+ */
+void (*clean)(BlockJob *job);
+
+/**
* If the callback is not NULL, it will be invoked when the job transitions
* into the paused state. Paused jobs must not perform any asynchronous
* I/O or event loop activity. This callback is used to quiesce jobs.
--
2.7.4
[Qemu-devel] [PATCH v3 3/6] blockjob: add .start field
Add an explicit start field to specify the entrypoint. We already have
ownership of the coroutine itself AND managing the lifetime of the
coroutine, let's take control of creation of the coroutine, too.
This will allow us to delay creation of the actual coroutine until we
know we'll actually start a BlockJob in block_job_start. This avoids
the sticky question of how to "un-create" a Coroutine that hasn't been
started yet.
Signed-off-by: John Snow
---
block/backup.c | 25 +
block/commit.c | 3 ++-
block/mirror.c | 4 +++-
block/stream.c | 3 ++-
include/block/blockjob_int.h | 3 +++
5 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 734a24c..4ed4494 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -323,17 +323,6 @@ static void backup_drain(BlockJob *job)
}
}
-static const BlockJobDriver backup_job_driver = {
-.instance_size = sizeof(BackupBlockJob),
-.job_type = BLOCK_JOB_TYPE_BACKUP,
-.set_speed = backup_set_speed,
-.commit = backup_commit,
-.abort = backup_abort,
-.clean = backup_clean,
-.attached_aio_context = backup_attached_aio_context,
-.drain = backup_drain,
-};
-
static BlockErrorAction backup_error_action(BackupBlockJob *job,
bool read, int error)
{
@@ -542,6 +531,18 @@ static void coroutine_fn backup_run(void *opaque)
block_job_defer_to_main_loop(&job->common, backup_complete, data);
}
+static const BlockJobDriver backup_job_driver = {
+.instance_size = sizeof(BackupBlockJob),
+.job_type = BLOCK_JOB_TYPE_BACKUP,
+.start = backup_run,
+.set_speed = backup_set_speed,
+.commit = backup_commit,
+.abort = backup_abort,
+.clean = backup_clean,
+.attached_aio_context = backup_attached_aio_context,
+.drain = backup_drain,
+};
+
void backup_start(const char *job_id, BlockDriverState *bs,
BlockDriverState *target, int64_t speed,
MirrorSyncMode sync_mode, BdrvDirtyBitmap *sync_bitmap,
@@ -653,7 +654,7 @@ void backup_start(const char *job_id, BlockDriverState *bs,
block_job_add_bdrv(&job->common, target);
job->common.len = len;
-job->common.co = qemu_coroutine_create(backup_run, job);
+job->common.co = qemu_coroutine_create(job->common.driver->start, job);
block_job_txn_add_job(txn, &job->common);
qemu_coroutine_enter(job->common.co);
return;
diff --git a/block/commit.c b/block/commit.c
index e1eda89..20d27e2 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -205,6 +205,7 @@ static const BlockJobDriver commit_job_driver = {
.instance_size = sizeof(CommitBlockJob),
.job_type = BLOCK_JOB_TYPE_COMMIT,
.set_speed = commit_set_speed,
+.start = commit_run,
};
void commit_start(const char *job_id, BlockDriverState *bs,
@@ -288,7 +289,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
s->backing_file_str = g_strdup(backing_file_str);
s->on_error = on_error;
-s->common.co = qemu_coroutine_create(commit_run, s);
+s->common.co = qemu_coroutine_create(s->common.driver->start, s);
trace_commit_start(bs, base, top, s, s->common.co);
qemu_coroutine_enter(s->common.co);
diff --git a/block/mirror.c b/block/mirror.c
index b2c1fb8..659e09c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -920,6 +920,7 @@ static const BlockJobDriver mirror_job_driver = {
.instance_size = sizeof(MirrorBlockJob),
.job_type = BLOCK_JOB_TYPE_MIRROR,
.set_speed = mirror_set_speed,
+.start = mirror_run,
.complete = mirror_complete,
.pause = mirror_pause,
.attached_aio_context = mirror_attached_aio_context,
@@ -930,6 +931,7 @@ static const BlockJobDriver commit_active_job_driver = {
.instance_size = sizeof(MirrorBlockJob),
.job_type = BLOCK_JOB_TYPE_COMMIT,
.set_speed = mirror_set_speed,
+.start = mirror_run,
.complete = mirror_complete,
.pause = mirror_pause,
.attached_aio_context = mirror_attached_aio_context,
@@ -1007,7 +1009,7 @@ static void mirror_start_job(const char *job_id,
BlockDriverState *bs,
}
}
-s->common.co = qemu_coroutine_create(mirror_run, s);
+s->common.co = qemu_coroutine_create(s->common.driver->start, s);
trace_mirror_start(bs, s, s->common.co, opaque);
qemu_coroutine_enter(s->common.co);
}
diff --git a/block/stream.c b/block/stream.c
index b05856b..92309ff 100644
--- a/block/stream.c
+++ b/block/stream.c
@@
[Qemu-devel] [PATCH v3 1/6] blockjob: fix dead pointer in txn list
From: Vladimir Sementsov-Ogievskiy
Though it is not intended to be reached through normal circumstances,
if we do not gracefully deconstruct the transaction QLIST, we may wind
up with stale pointers in the list.
The rest of this series attempts to address the underlying issues,
but this should fix list inconsistencies.
Signed-off-by: Vladimir Sementsov-Ogievskiy
Tested-by: John Snow
Reviewed-by: John Snow
[Rewrote commit message. --js]
Signed-off-by: John Snow
Reviewed-by: Eric Blake
Reviewed-by: Kevin Wolf
Signed-off-by: John Snow
---
blockjob.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/blockjob.c b/blockjob.c
index 4aa14a4..4d0ef53 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -256,6 +256,7 @@ static void block_job_completed_single(BlockJob *job)
}
if (job->txn) {
+QLIST_REMOVE(job, txn_list);
block_job_txn_unref(job->txn);
}
block_job_unref(job);
--
2.7.4
Re: [Qemu-devel] [PATCH v9 00/12] virtio-crypto: introduce framework and device emulation
On 10/31/2016 03:52 AM, Gonglei (Arei) wrote: >> > Unfortunately I encountered a problem already on x86_64, more precisely >> > the guest hangs in virtio_crypto_alg_ablkcipher_init_session after the >> > kick. But before I start explaining in detail let me ask you: what are >> > your plans for the linux kernel driver? I used the most current version >> > from your github, but also tried to look for patches on mailing lists >> > and I have found none. > I haven't submit the virtio-crypto driver to the mailing list yet cause > I wanted to the patches on the QEMU side can be merged firstly, > and then focus to the frontend driver. > That is fine and the QEMU stuff does look good :). I'm still curious about your road map that is what should I expect on the kernel side and when? You could also cc me ;) when sending to the list. >> > IMHO the problem causing my hang is in the kernel >> > module. But I do not know here am I supposed to comment on it -- so I'm >> > commenting here. Furthermore your kernel module currently do not seem to >> > care about endianness which is bad for s390x. >> > > That's true. > For the sake of the experiment I have reworked virtio_crypto_alg_ablkcipher_init_session so that id does care about the endianness of the guest. With that the functions seems to work on s390x (but of course the next byte order problem warrants that encryption/decryption does not work yet). It is not beautiful but I can give you a patch if you want. >> > The hangs basically boils down to the following: after the kick in >> > virtio_crypto_alg_ablkcipher_init_session we reach the point (in QEMU) >> > where we want to translate the address for the third descriptor, the one >> > which designates a buffer for the virtio_crypto_session_input. My >> > backtrace looks like this: >> > >> > >> > #0 address_space_translate (as=, >> > as@entry=0x5607d700 , >> > addr=addr@entry=71468256835928, xlat=xlat@entry=0x7fffef4dc1a0, >> > plen=plen@entry=0x7fffef4dc198, is_write=is_write@entry=true) >> > at /home/pasic/devel/qemu/exec.c:481 >> > #1 0x5575ef19 in address_space_map (as=0x5607d700 >> > , addr=71468256835928, plen=, >> > is_write=) at /home/pasic/devel/qemu/exec.c:2927 >> > #2 0x557e5d00 in virtqueue_map_desc (vdev=0x57d80de0, >> > p_num_sg=0x7fffef4dc2bc, addr=0x7fffef4dc2f0, iov=0x7fffef4de300, >> > max_num_sg=1022, is_write=true, pa=71468256835928, sz=16) >> > at /home/pasic/devel/qemu/hw/virtio/virtio.c:558 >> > #3 0x557e5f86 in virtqueue_pop (vq=0x7fffef4dc2c0, >> > sz=93825003936576) at /home/pasic/devel/qemu/hw/virtio/virtio.c:717 >> > #4 0x557ed80b in virtio_crypto_handle_ctrl (vdev=, >> > vq=0x7fffeec3e090) at /home/pasic/devel/qemu/hw/virtio/virtio-crypto.c:218 >> > >> > Now the first suspicious thing I see is that address_space_translate >> > returns a pointer to the memory region io_mem_unassigned. Next thing >> > happening is that we take the !memory_access_is_direct(mr, is_write) >> > branch in address_space_map and return NULL at line 2932 which however >> > makes virtqueue_map_desc report "qemu-system-x86_64: virtio: bogus >> > descriptor or out of resources" and return with false. Then pop returns >> > null too and spins virtio_crypto_alg_ablkcipher_init_session till the >> > end of time waiting for the answer. >> > > It seems that the frontend driver passed a invalid gpa to the QEMU side, > The Qemu side will invokes virtio_error() to notify the guest to reset the > device. > I did not say previously but I have checked and the gpa is good, that is it has a correct offset in respect to ctrl's gpa (the next variable on stack, which is the previous desc.addr used read only) which is good. Furthermore the issue seems to be platform dependent as I do not see this on s390x. >> > Now if I change the code so that the virtio_crypto_session_input >> > instance's backing memory is allocated with kzalloc(sizeof(*input), >> > GFP_DMA|GFP_NOIO) of allocating the on the stack things start working >> > for me. I'm not too deep into this yet, so please forgive me if >> > I'm a bit vague. >> > > Good, maybe you find a bug of the frontend driver. Using stack memory > might not be a good idea ;) > Yeah, as I said, can't say (that is my understanding too shallow). Cheers, Halil
Re: [Qemu-devel] [PATCH] hw/xen/xen_pvdev: Include qemu/log.h for qemu_log_vprintf()
On Wed, 2 Nov 2016, Thomas Huth wrote: > Olaf Hering reported a build failure due to an undefined reference > to 'qemu_log_vprintf'. Explicitely including qemu/log.h seems to > fix the issue. > > Signed-off-by: Thomas Huth The fix makes sense: Acked-by: Stefano Stabellini However I was unable to reproduce the build failure, so I would like a confirmation from Olaf that the fix is working. > hw/xen/xen_pvdev.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/xen/xen_pvdev.c b/hw/xen/xen_pvdev.c > index 405e154..5212bc6 100644 > --- a/hw/xen/xen_pvdev.c > +++ b/hw/xen/xen_pvdev.c > @@ -18,7 +18,7 @@ > */ > > #include "qemu/osdep.h" > - > +#include "qemu/log.h" > #include "hw/xen/xen_backend.h" > #include "hw/xen/xen_pvdev.h" > > -- > 1.8.3.1 >
Re: [Qemu-devel] [PULL 0/2] M68k for 2.8 patches
Le 02/11/2016 à 18:23, Stefan Hajnoczi a écrit : > On Wed, Nov 2, 2016 at 5:17 PM, Stefan Hajnoczi wrote: >> On Wed, Nov 2, 2016 at 5:12 PM, Laurent Vivier wrote: >>> Le 02/11/2016 à 18:07, Stefan Hajnoczi a écrit : On Mon, Oct 31, 2016 at 10:50:29AM +0100, Laurent Vivier wrote: > The following changes since commit > 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: > > Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161028-1' > into staging (2016-10-28 17:59:04 +0100) > > are available in the git repository at: > > g...@github.com:vivier/qemu-m68k.git tags/m68k-for-2.8-pull-request This URI is not publicly accessible. Some tools will fail to apply your pull request because of this. Please adjust your .gitconfig: [remote "origin"] url = git://github.com/vivier/qemu.git pushurl = g...@github.com:vivier/qemu.git >>> >>> Thank you Stefan. >>> >>> I'm using your tool "git-publish", is there a way to detect this problem >>> automatically? >> >> Right now git-publish doesn't check the generated URI. In some use >> cases it's reasonable to use an SSH URI so adding an error or even >> warning to git-publish isn't ideal. >> >> I am modifying the patches tool (https://github.com/stefanha/patches) >> to automatically translate GitHub URIs since they are so prevalent... >> This will allow me to apply pull requests that have GitHub SSH URIs. > > On second thought, even that isn't a general solution. If people are > collaborating on private GitHub repos then translating URIs to HTTPS > is not correct either. > > Instead of trying to do magic I'll ask pull request submitters to fix > their .gitconfig. Perhaps you can also update git-publish README.rst? Thanks, Laurent
Re: [Qemu-devel] [PULL 0/2] M68k for 2.8 patches
On Wed, Nov 2, 2016 at 5:17 PM, Stefan Hajnoczi wrote: > On Wed, Nov 2, 2016 at 5:12 PM, Laurent Vivier wrote: >> Le 02/11/2016 à 18:07, Stefan Hajnoczi a écrit : >>> On Mon, Oct 31, 2016 at 10:50:29AM +0100, Laurent Vivier wrote: The following changes since commit 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161028-1' into staging (2016-10-28 17:59:04 +0100) are available in the git repository at: g...@github.com:vivier/qemu-m68k.git tags/m68k-for-2.8-pull-request >>> >>> This URI is not publicly accessible. Some tools will fail to apply your >>> pull request because of this. >>> >>> Please adjust your .gitconfig: >>> >>> [remote "origin"] >>> url = git://github.com/vivier/qemu.git >>> pushurl = g...@github.com:vivier/qemu.git >> >> Thank you Stefan. >> >> I'm using your tool "git-publish", is there a way to detect this problem >> automatically? > > Right now git-publish doesn't check the generated URI. In some use > cases it's reasonable to use an SSH URI so adding an error or even > warning to git-publish isn't ideal. > > I am modifying the patches tool (https://github.com/stefanha/patches) > to automatically translate GitHub URIs since they are so prevalent... > This will allow me to apply pull requests that have GitHub SSH URIs. On second thought, even that isn't a general solution. If people are collaborating on private GitHub repos then translating URIs to HTTPS is not correct either. Instead of trying to do magic I'll ask pull request submitters to fix their .gitconfig. Stefan
Re: [Qemu-devel] Mysterious RST connection with virtio-net NATting VM
On October 21, 2016 04:34:38 PM Neil Skrypuch wrote: > I have a NATting VM (let's call this vm/nat) sitting in front of another VM > (let's call this one vm/wget), with vm/wget residing in a private virtual > network, with all network connectivity for vm/wget going through vm/nat. > Additionally, I have a web server running on a physical machine from which > vm/wget is trying to fetch a file. Graphically, it looks something like > this: > > web server <-> vm/nat <-> vm/wget > > Now, when vm/wget tries to wget a file from the web server, it frequently > gets a connection reset by peer network error, wget will retry the > connection and resume the download, after which the cycle will repeat many > times before the file eventually fully downloads. The problem is not > limited to wget though, both scp and curl experience the same connection > reset by peer issue quite consistently. > > After some collaborative debugging on IRC, tcpdump revealed that an RST is > being generated which is breaking the network connection. Now, from the web > server's point of view, the RST is coming from vm/wget and from vm/wget's > point of view, the RST is coming from the web server. Further tcpdumping > from both NICs on vm/nat reveals that vm/nat itself is generating the RSTs, > although it is not clear why. Furthermore, the TCP sequence numbers on the > RSTs are totally out of line with the ACKs in the stream, for example: > > 13:01:19.819882 IP 192.168.100.160.37675 > 192.168.123.198.80: Flags [.], > ack 1914061, win 65535, length 0 > 13:01:19.819886 IP 192.168.100.160.37675 > 192.168.123.198.80: Flags [.], > ack 1914061, win 65535, length 0 > 13:01:19.819889 IP 192.168.100.160.37675 > 192.168.123.198.80: Flags [.], > ack 1918441, win 65535, length 0 > 13:01:19.820127 IP 192.168.123.198.80 > 192.168.100.160.37675: Flags [R], > seq 1390832295, win 0, length 0 > > We ran out of ideas on how to further debug the RST issue, but ideas are > welcome. > > One other thing worth noting is that vm/nat is configured with a pair of > virtio-net NICs. If I change the NICs to e1000 models, the RST issue > vanishes. More specifically, only the NIC facing the web server on vm/nat > needs to be changed to e1000 for the issue to go away, the other can remain > virtio-net. Also, running a wget directly on vm/nat works in all cases. I > tried a number of other things to try and narrow down the issue, none of > which helped, including: > > - vhost=on > - qemu versions 2.7.0, 2.3.0 and 2.0.0 > - disabling kvm accel (ie, using TCG) > > The full command line for vm/nat is this: > > /usr/bin/qemu-system-x86_64 -machine accel=kvm -vga cirrus -m 512 -smp > 4,cores=4,sockets=1,threads=1 -drive > file=/var/lib/kvm/lb.polldev.com.img,format=raw,if=virtio -device > virtio-net- pci,mac=52:54:00:42:45:15,netdev=net1 -netdev > type=tap,script=/etc/qemu-ifup- br0,downscript=no,id=net1 -device > virtio-net- > pci,mac=52:54:00:42:45:16,netdev=net2 -netdev > type=tap,script=/etc/qemu-ifup- br2,downscript=no,id=net2 -curses -monitor > unix:/var/lib/kvm/monitor/lb,server,nowait -name > "lb.polldev.com",process=lb.polldev.com > > I was able to reproduce this with different web server machines, as well as > different vm/wget machines, as well as when moving vm/nat to a different > host (albeit for the other host, the RST issue presents itself > significantly less frequently). > > Since the problem goes away with e1000, my gut feeling is that virtio-net is > somehow slightly mangling the packets which causes iptables to eventually > RST the connection, although it's probably more subtle than that, since I'm > sure I'm not the first person to try NAT in a virtio-net VM. > > Any thoughts? > > - Neil I was able to narrow this down a bit. In particular, it only happens when vm/wget has net.ipv4.tcp_window_scaling = 0. Once I restore window scaling to the default value (1), I no longer get the mysterious RSTs and everything works as expected. We actually don't have a good reason to disable window scaling in the first place (in fact it's pretty much required to get close to wire line speeds on most modern networks), so I'm considering this a solution. Though, it would be interesting to know how disabled window scaling causes the RST, there is almost certainly a bug there, somewhere. - Neil
Re: [Qemu-devel] [PULL 0/2] M68k for 2.8 patches
On Wed, Nov 2, 2016 at 5:12 PM, Laurent Vivier wrote: > Le 02/11/2016 à 18:07, Stefan Hajnoczi a écrit : >> On Mon, Oct 31, 2016 at 10:50:29AM +0100, Laurent Vivier wrote: >>> The following changes since commit 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: >>> >>> Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161028-1' >>> into staging (2016-10-28 17:59:04 +0100) >>> >>> are available in the git repository at: >>> >>> g...@github.com:vivier/qemu-m68k.git tags/m68k-for-2.8-pull-request >> >> This URI is not publicly accessible. Some tools will fail to apply your >> pull request because of this. >> >> Please adjust your .gitconfig: >> >> [remote "origin"] >> url = git://github.com/vivier/qemu.git >> pushurl = g...@github.com:vivier/qemu.git > > Thank you Stefan. > > I'm using your tool "git-publish", is there a way to detect this problem > automatically? Right now git-publish doesn't check the generated URI. In some use cases it's reasonable to use an SSH URI so adding an error or even warning to git-publish isn't ideal. I am modifying the patches tool (https://github.com/stefanha/patches) to automatically translate GitHub URIs since they are so prevalent... This will allow me to apply pull requests that have GitHub SSH URIs. Stefan
Re: [Qemu-devel] [PULL 0/2] M68k for 2.8 patches
Le 02/11/2016 à 18:07, Stefan Hajnoczi a écrit : > On Mon, Oct 31, 2016 at 10:50:29AM +0100, Laurent Vivier wrote: >> The following changes since commit 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: >> >> Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161028-1' into >> staging (2016-10-28 17:59:04 +0100) >> >> are available in the git repository at: >> >> g...@github.com:vivier/qemu-m68k.git tags/m68k-for-2.8-pull-request > > This URI is not publicly accessible. Some tools will fail to apply your > pull request because of this. > > Please adjust your .gitconfig: > > [remote "origin"] > url = git://github.com/vivier/qemu.git > pushurl = g...@github.com:vivier/qemu.git Thank you Stefan. I'm using your tool "git-publish", is there a way to detect this problem automatically? Laurent signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PULL 0/2] M68k for 2.8 patches
On Mon, Oct 31, 2016 at 10:50:29AM +0100, Laurent Vivier wrote: > The following changes since commit 5b2ecabaeabc17f032197246c4846b9ba95ba8a6: > > Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20161028-1' into > staging (2016-10-28 17:59:04 +0100) > > are available in the git repository at: > > g...@github.com:vivier/qemu-m68k.git tags/m68k-for-2.8-pull-request This URI is not publicly accessible. Some tools will fail to apply your pull request because of this. Please adjust your .gitconfig: [remote "origin"] url = git://github.com/vivier/qemu.git pushurl = g...@github.com:vivier/qemu.git Thanks, Stefan signature.asc Description: PGP signature
Re: [Qemu-devel] [QEMU PATCH v10 2/3] migration: migrate QTAILQ
On 11/02/2016 03:45 AM, Juan Quintela wrote:
> Jianjun Duan wrote:
>> Currently we cannot directly transfer a QTAILQ instance because of the
>> limitation in the migration code. Here we introduce an approach to
>> transfer such structures. We created VMStateInfo vmstate_info_qtailq
>> for QTAILQ. Similar VMStateInfo can be created for other data structures
>> such as list.
>>
>> This approach will be used to transfer pending_events and ccs_list in spapr
>> state.
>>
>> We also create some macros in qemu/queue.h to access a QTAILQ using pointer
>> arithmetic. This ensures that we do not depend on the implementation
>> details about QTAILQ in the migration code.
>>
>> Signed-off-by: Jianjun Duan
>
>
>> +
>> +trace_get_qtailq(vmsd->name, version_id);
>> +if (version_id > vmsd->version_id) {
>> +error_report("%s %s", vmsd->name, "too new");
>> +trace_get_qtailq_end(vmsd->name, "too new", -EINVAL);
>> +
>> +return -EINVAL;
>> +}
>> +if (version_id < vmsd->minimum_version_id) {
>> +error_report("%s %s", vmsd->name, "too old");
>> +trace_get_qtailq_end(vmsd->name, "too old", -EINVAL);
>> +return -EINVAL;
>> +}
>> +
>> +while (qemu_get_byte(f)) {
>> +elm = g_malloc(size);
>
> I think this is not generic enough. We really need to allocate a new
> element, and then fill it with default values.
>
> virtio list code use it in this way.
>
>
To do that we need probably to expand VMStateDescription and/or
VMStateField so that a default value can be supplied. Or we can always
fill the untouched fields in post_load.
Thanks,
Jianjun
> Thanks, Juan.
>
Re: [Qemu-devel] [Qemu-block] [PULL v2 00/14] Block patches for 2.8
On Tue, Nov 01, 2016 at 08:50:57AM -0400, Jeff Cody wrote: > The following changes since commit bf99fd3983d7185178a0f65ce29bb94b1aecaed1: > > Merge remote-tracking branch 'remotes/rth/tags/pull-sparc-20161031-2' into > staging (2016-11-01 11:21:02 +) > > are available in the git repository at: > > g...@github.com:codyprime/qemu-kvm-jtc.git tags/block-pull-request This pull request does not have a publicly-accessible URI. Please adjust your .gitconfig: [remote "public"] url = git://github.com/codyprime/qemu.git pushurl = g...@github.com:codyprime/qemu.git Thanks, Stefan signature.asc Description: PGP signature
[Qemu-devel] [PATCH v1 1/2] arm_generic_timer: Add the ARM Generic Timer
Add the ARM generic timer. This allows the guest to poll the timer for
values and also supports secure writes only.
Signed-off-by: Alistair Francis
---
hw/timer/Makefile.objs | 1 +
hw/timer/arm_generic_timer.c | 216 +++
include/hw/timer/arm_generic_timer.h | 60 ++
3 files changed, 277 insertions(+)
create mode 100644 hw/timer/arm_generic_timer.c
create mode 100644 include/hw/timer/arm_generic_timer.h
diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index 7ba8c23..f88c468 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -17,6 +17,7 @@ common-obj-$(CONFIG_IMX) += imx_epit.o
common-obj-$(CONFIG_IMX) += imx_gpt.o
common-obj-$(CONFIG_LM32) += lm32_timer.o
common-obj-$(CONFIG_MILKYMIST) += milkymist-sysctl.o
+common-obj-$(CONFIG_XLNX_ZYNQMP) += arm_generic_timer.o
obj-$(CONFIG_EXYNOS4) += exynos4210_mct.o
obj-$(CONFIG_EXYNOS4) += exynos4210_pwm.o
diff --git a/hw/timer/arm_generic_timer.c b/hw/timer/arm_generic_timer.c
new file mode 100644
index 000..8341e06
--- /dev/null
+++ b/hw/timer/arm_generic_timer.c
@@ -0,0 +1,216 @@
+/*
+ * QEMU model of the ARM Generic Timer
+ *
+ * Copyright (c) 2016 Xilinx Inc.
+ * Written by Alistair Francis
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/timer/arm_generic_timer.h"
+#include "qemu/timer.h"
+#include "qemu/log.h"
+
+#ifndef ARM_GEN_TIMER_ERR_DEBUG
+#define ARM_GEN_TIMER_ERR_DEBUG 0
+#endif
+
+static void counter_control_postw(RegisterInfo *reg, uint64_t val64)
+{
+ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
+bool new_status = extract32(s->regs[R_COUNTER_CONTROL_REGISTER],
+R_COUNTER_CONTROL_REGISTER_EN_SHIFT,
+R_COUNTER_CONTROL_REGISTER_EN_LENGTH);
+uint64_t current_ticks;
+
+current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ NANOSECONDS_PER_SECOND, 100);
+
+if ((s->enabled && !new_status) ||
+(!s->enabled && new_status)) {
+/* The timer is being disabled or enabled */
+s->tick_offset = current_ticks - s->tick_offset;
+}
+
+s->enabled = new_status;
+}
+
+static uint64_t couter_low_value_postr(RegisterInfo *reg, uint64_t val64)
+{
+ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
+uint64_t current_ticks, total_ticks;
+uint32_t low_ticks;
+
+if (s->enabled) {
+current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ NANOSECONDS_PER_SECOND, 100);
+total_ticks = current_ticks - s->tick_offset;
+low_ticks = (uint32_t) total_ticks;
+} else {
+/* Timer is disabled, return the time when it was disabled */
+low_ticks = (uint32_t) s->tick_offset;
+}
+
+return low_ticks;
+}
+
+static uint64_t couter_high_value_postr(RegisterInfo *reg, uint64_t val64)
+{
+ARMGenTimer *s = ARM_GEN_TIMER(reg->opaque);
+uint64_t current_ticks, total_ticks;
+uint32_t high_ticks;
+
+if (s->enabled) {
+current_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
+ NANOSECONDS_PER_SECOND, 100);
+total_ticks = current_ticks - s->tick_offset;
+high_ticks = (uint32_t) (total_ticks >> 32);
+} else {
+/* Timer is disabled, return the time when it was disabled */
+high_ticks = (uint32_t) (s->tick_offset >> 32);
+}
+
+return high_ticks;
+}
+
+
+static RegisterAccessInfo arm_gen_timer_regs_info[] = {
+{ .name = "COUNTER_CONTROL_REGISTER",
+.addr = A_COUNTER_CONTROL_REGISTER,
+.rsvd = 0xfffc,
+.post_write = counter_control_postw,
+},{ .name = "COUNTER_STATUS_REGISTER",
+.addr = A_COUNTER_STATUS_REGISTER,
+.rsvd = 0xfffd, .ro = 0x2,
+},{ .name = "CURRENT_COUNTER_VALUE_LOWER_REGISTER",
+
Re: [Qemu-devel] [QEMU PATCH v10 1/3] migration: extend VMStateInfo
On 11/02/2016 03:40 AM, Juan Quintela wrote:
> Jianjun Duan wrote:
>> Current migration code cannot handle some data structures such as
>> QTAILQ in qemu/queue.h. Here we extend the signatures of put/get
>> in VMStateInfo so that customized handling is supported.
>>
>> Signed-off-by: Jianjun Duan
>
>> +/* VMStateInfo allows customized migration of objects that don't fit in
>> + * any category in VMStateFlags. Additional information can be passed
>> + * into get and put in terms of field and vmdesc parameters.
>> + * For primitive data types such as integer, field and vmdesc parameters
>> + * should be ignored inside get/put. */
>> struct VMStateInfo {
>> const char *name;
>> -int (*get)(QEMUFile *f, void *pv, size_t size);
>> -void (*put)(QEMUFile *f, void *pv, size_t size);
>> +int (*get)(QEMUFile *f, void *pv, size_t size, VMStateField *field);
>> +void (*put)(QEMUFile *f, void *pv, size_t size, VMStateField *field,
>> +QJSON *vmdesc);
>
> If we are changing put type, I would like to add an error value to it.
>
so we change the return type to int?
>> static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription
>> *vmsd,
>> void *opaque, QJSON *vmdesc);
>> @@ -83,6 +84,7 @@ int vmstate_load_state(QEMUFile *f, const
>> VMStateDescription *vmsd,
>>
>> trace_vmstate_load_state(vmsd->name, version_id);
>> if (version_id > vmsd->version_id) {
>> +error_report("%s %s", vmsd->name, "too new");
>> trace_vmstate_load_state_end(vmsd->name, "too new", -EINVAL);
>> return -EINVAL;
>> }
>> @@ -93,6 +95,7 @@ int vmstate_load_state(QEMUFile *f, const
>> VMStateDescription *vmsd,
>> trace_vmstate_load_state_end(vmsd->name, "old path", ret);
>> return ret;
>> }
>> +error_report("%s %s", vmsd->name, "too old");
>> trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL);
>> return -EINVAL;
>> }
>
> This two are good, but don't belong to this patch, please sent separately.
>
OK.
>> @@ -122,8 +125,10 @@ int vmstate_load_state(QEMUFile *f, const
>> VMStateDescription *vmsd,
>> ret = vmstate_load_state(f, field->vmsd, addr,
>> field->vmsd->version_id);
>> } else {
>> -ret = field->info->get(f, addr, size);
>> -
>> +/* field is always passed in. But it should be ignored
>> by
>> + * get when not needed. It is only needed in cases* of
>> + * customized handling, such as migrating QTAILQ. */
>> +ret = field->info->get(f, addr, size, field);
>
> I would not put this comment here, better on the header when the field
> is declared? Same for the next one.
>
OK.
Thanks,
Jianjun
>> }
>> if (ret >= 0) {
>> ret = qemu_file_get_error(f);
>> @@ -328,7 +333,11 @@ void vmstate_save_state(QEMUFile *f, const
>> VMStateDescription *vmsd,
>> if (field->flags & VMS_STRUCT) {
>> vmstate_save_state(f, field->vmsd, addr, vmdesc_loop);
>> } else {
>> -field->info->put(f, addr, size);
>> +/* field and vmdesc_loop are always passed in. But they
>> + * should be ignored by put when not needed. They are
>> + * only needed in cases f customized handling, such as
>> + * migrating QTAILQ. */
>> +field->info->put(f, addr, size, field, vmdesc_loop);
>> }
>
>
> Rest of patch is ok.
>
[Qemu-devel] [PATCH v3 3/3] qemu-doc: update gluster protocol usage guide
Document:
1. The new debug and logfile options with their usages
2. New json format and its usage and
3. update "GlusterFS, Device URL Syntax" section in "Invocation"
Signed-off-by: Prasanna Kumar Kalever
---
qemu-doc.texi | 59 +++--
qemu-options.hx | 25 ++--
2 files changed, 68 insertions(+), 16 deletions(-)
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 023c140..02cb39d 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -1041,35 +1041,55 @@ GlusterFS is an user space distributed file system.
You can boot from the GlusterFS disk image with the command:
@example
-qemu-system-x86_64 -drive
file=gluster[+@var{transport}]://[@var{server}[:@var{port}]]/@var{volname}/@var{image}[?socket=...]
+URI:
+qemu-system-x86_64 -drive
file=gluster[+@var{type}]://[@var{host}[:@var{port}]]/@var{volume}/@var{path}
+ [?socket=...][,file.debug=9][,file.logfile=...]
+
+JSON:
+qemu-system-x86_64 'json:@{"driver":"qcow2",
+ "file":@{"driver":"gluster",
+
"volume":"testvol","path":"a.img","debug":9,"logfile":"...",
+
"server":[@{"type":"tcp","host":"...","port":"..."@},
+
@{"type":"unix","socket":"..."@}]@}@}'
@end example
@var{gluster} is the protocol.
-@var{transport} specifies the transport type used to connect to gluster
+@var{type} specifies the transport type used to connect to gluster
management daemon (glusterd). Valid transport types are
-tcp, unix and rdma. If a transport type isn't specified, then tcp
-type is assumed.
+tcp and unix. In the URI form, if a transport type isn't specified,
+then tcp type is assumed.
-@var{server} specifies the server where the volume file specification for
-the given volume resides. This can be either hostname, ipv4 address
-or ipv6 address. ipv6 address needs to be within square brackets [ ].
-If transport type is unix, then @var{server} field should not be specified.
+@var{host} specifies the server where the volume file specification for
+the given volume resides. This can be either a hostname or an ipv4 address.
+If transport type is unix, then @var{host} field should not be specified.
Instead @var{socket} field needs to be populated with the path to unix domain
socket.
@var{port} is the port number on which glusterd is listening. This is optional
-and if not specified, QEMU will send 0 which will make gluster to use the
-default port. If the transport type is unix, then @var{port} should not be
-specified.
+and if not specified, it defaults to port 24007. If the transport type is unix,
+then @var{port} should not be specified.
+
+@var{volume} is the name of the gluster volume which contains the disk image.
+
+@var{path} is the path to the actual disk image that resides on gluster volume.
+
+@var{debug} is the logging level of the gluster protocol driver. Debug levels
+are 0-9, with 9 being the most verbose, and 0 representing no debugging output.
+The default level is 4. The current logging levels defined in the gluster
source
+are 0 - None, 1 - Emergency, 2 - Alert, 3 - Critical, 4 - Error, 5 - Warning,
+6 - Notice, 7 - Info, 8 - Debug, 9 - Trace
+
+@var{logfile} is a commandline option to mention log file path which helps in
+logging to the specified file and also help in persisting the gfapi logs. The
+default is stderr.
+
-@var{volname} is the name of the gluster volume which contains the disk image.
-@var{image} is the path to the actual disk image that resides on gluster
volume.
You can create a GlusterFS disk image with the command:
@example
-qemu-img create gluster://@var{server}/@var{volname}/@var{image} @var{size}
+qemu-img create gluster://@var{host}/@var{volume}/@var{path} @var{size}
@end example
Examples
@@ -1082,6 +1102,17 @@ qemu-system-x86_64 -drive
file=gluster+tcp://[1:2:3:4:5:6:7:8]:24007/testvol/dir
qemu-system-x86_64 -drive
file=gluster+tcp://server.domain.com:24007/testvol/dir/a.img
qemu-system-x86_64 -drive
file=gluster+unix:///testvol/dir/a.img?socket=/tmp/glusterd.socket
qemu-system-x86_64 -drive file=gluster+rdma://1.2.3.4:24007/testvol/a.img
+qemu-system-x86_64 -drive
file=gluster://1.2.3.4/testvol/a.img,file.debug=9,file.logfile=/var/log/qemu-gluster.log
+qemu-system-x86_64 'json:@{"driver":"qcow2",
+ "file":@{"driver":"gluster",
+"volume":"testvol","path":"a.img",
+
"debug":9,"logfile":"/var/log/qemu-gluster.log",
+
"server":[@{"type":"tcp","host":"1.2.3.4","port":24007@},
+
@{"type":"unix","socket":"/var/run/glusterd.socket"@}]@}@}'
+qemu-system-x86_64 -drive
driver=qcow2,file.driver=gluster,file.volume=testvol,file.path=/path/a.img,
+
file.debug=9,
[Qemu-devel] [PATCH v3 0/3] qemu-doc: update gluster protocol usage guide
v3: Address review comments by Eric Blake This version split to 3 patches Patch 1/3: Fix QMP definition of BlockdevOptionsGluster, change s/debug-level/debug/ Patch 2/3: Fix QMP definition of BlockdevOptionsNfs, change s/debug-level/debug/ Patch 3/3: no changes to one in v2 v2: Address review comments by Eric Blake on v1 Mostly grammar related changes, formating for better readability and updated commit message v1: Initial commit Prasanna Kumar Kalever (3): block/gluster: fix QMP to match debug option block/nfs: fix QMP to match debug option qemu-doc: update gluster protocol usage guide block/gluster.c | 34 +++--- block/nfs.c | 4 ++-- qapi/block-core.json | 8 +++ qemu-doc.texi| 59 +++- qemu-options.hx | 25 -- 5 files changed, 91 insertions(+), 39 deletions(-) -- 2.7.4
[Qemu-devel] [PATCH v3 2/3] block/nfs: fix QMP to match debug option
The QMP definition of BlockdevOptionsNfs:
{ 'struct': 'BlockdevOptionsNfs',
'data': { 'server': 'NFSServer',
'path': 'str',
'*user': 'int',
'*group': 'int',
'*tcp-syn-count': 'int',
'*readahead-size': 'int',
'*page-cache-size': 'int',
'*debug-level': 'int' } }
To make this consistent with other block protocols like gluster, lets
change s/debug-level/debug/
Suggested-by: Eric Blake
Signed-off-by: Prasanna Kumar Kalever
---
block/nfs.c | 4 ++--
qapi/block-core.json | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/nfs.c b/block/nfs.c
index 55c4e0b..21f3c8c 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -134,7 +134,7 @@ static int nfs_parse_uri(const char *filename, QDict
*options, Error **errp)
qdict_put(options, "page-cache-size",
qstring_from_str(qp->p[i].value));
} else if (!strcmp(qp->p[i].name, "debug")) {
-qdict_put(options, "debug-level",
+qdict_put(options, "debug",
qstring_from_str(qp->p[i].value));
} else {
error_setg(errp, "Unknown NFS parameter name: %s",
@@ -165,7 +165,7 @@ static bool nfs_has_filename_options_conflict(QDict
*options, Error **errp)
!strcmp(qe->key, "tcp-syn-count") ||
!strcmp(qe->key, "readahead-size") ||
!strcmp(qe->key, "page-cache-size") ||
-!strcmp(qe->key, "debug-level") ||
+!strcmp(qe->key, "debug") ||
strstart(qe->key, "server.", NULL))
{
error_setg(errp, "Option %s cannot be used with a filename",
diff --git a/qapi/block-core.json b/qapi/block-core.json
index a569cfb..804da61 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2292,7 +2292,7 @@
# @page-cache-size: #optional set the pagecache size in bytes (defaults
# to libnfs default)
#
-# @debug-level: #optional set the NFS debug level (max 2) (defaults
+# @debug: #optional set the NFS debug level (max 2) (defaults
# to libnfs default)
#
# Since 2.8
@@ -2305,7 +2305,7 @@
'*tcp-syn-count': 'int',
'*readahead-size': 'int',
'*page-cache-size': 'int',
-'*debug-level': 'int' } }
+'*debug': 'int' } }
##
# @BlockdevOptionsCurl
--
2.7.4
[Qemu-devel] [PATCH v3 1/3] block/gluster: fix QMP to match debug option
The QMP definition of BlockdevOptionsGluster:
{ 'struct': 'BlockdevOptionsGluster',
'data': { 'volume': 'str',
'path': 'str',
'server': ['GlusterServer'],
'*debug-level': 'int',
'*logfile': 'str' } }
But instead of 'debug-level we have exported 'debug' as the option for choosing
debug level of gluster protocol driver.
This patch fix QMP definition BlockdevOptionsGluster
s/debug-level/debug/
Suggested-by: Eric Blake
Signed-off-by: Prasanna Kumar Kalever
---
block/gluster.c | 34 +-
qapi/block-core.json | 4 ++--
2 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/block/gluster.c b/block/gluster.c
index 0ce15f7..e23dc3b 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -48,7 +48,7 @@ typedef struct BDRVGlusterState {
struct glfs_fd *fd;
char *logfile;
bool supports_seek_data;
-int debug_level;
+int debug;
} BDRVGlusterState;
typedef struct BDRVGlusterReopenState {
@@ -433,7 +433,7 @@ static struct glfs
*qemu_gluster_glfs_init(BlockdevOptionsGluster *gconf,
}
}
-ret = glfs_set_logging(glfs, gconf->logfile, gconf->debug_level);
+ret = glfs_set_logging(glfs, gconf->logfile, gconf->debug);
if (ret < 0) {
goto out;
}
@@ -787,17 +787,17 @@ static int qemu_gluster_open(BlockDriverState *bs, QDict
*options,
filename = qemu_opt_get(opts, GLUSTER_OPT_FILENAME);
-s->debug_level = qemu_opt_get_number(opts, GLUSTER_OPT_DEBUG,
+s->debug = qemu_opt_get_number(opts, GLUSTER_OPT_DEBUG,
GLUSTER_DEBUG_DEFAULT);
-if (s->debug_level < 0) {
-s->debug_level = 0;
-} else if (s->debug_level > GLUSTER_DEBUG_MAX) {
-s->debug_level = GLUSTER_DEBUG_MAX;
+if (s->debug < 0) {
+s->debug = 0;
+} else if (s->debug > GLUSTER_DEBUG_MAX) {
+s->debug = GLUSTER_DEBUG_MAX;
}
gconf = g_new0(BlockdevOptionsGluster, 1);
-gconf->debug_level = s->debug_level;
-gconf->has_debug_level = true;
+gconf->debug = s->debug;
+gconf->has_debug = true;
logfile = qemu_opt_get(opts, GLUSTER_OPT_LOGFILE);
s->logfile = g_strdup(logfile ? logfile : GLUSTER_LOGFILE_DEFAULT);
@@ -873,8 +873,8 @@ static int qemu_gluster_reopen_prepare(BDRVReopenState
*state,
qemu_gluster_parse_flags(state->flags, &open_flags);
gconf = g_new0(BlockdevOptionsGluster, 1);
-gconf->debug_level = s->debug_level;
-gconf->has_debug_level = true;
+gconf->debug = s->debug;
+gconf->has_debug = true;
gconf->logfile = g_strdup(s->logfile);
gconf->has_logfile = true;
reop_s->glfs = qemu_gluster_init(gconf, state->bs->filename, NULL, errp);
@@ -1010,14 +1010,14 @@ static int qemu_gluster_create(const char *filename,
char *tmp = NULL;
gconf = g_new0(BlockdevOptionsGluster, 1);
-gconf->debug_level = qemu_opt_get_number_del(opts, GLUSTER_OPT_DEBUG,
+gconf->debug = qemu_opt_get_number_del(opts, GLUSTER_OPT_DEBUG,
GLUSTER_DEBUG_DEFAULT);
-if (gconf->debug_level < 0) {
-gconf->debug_level = 0;
-} else if (gconf->debug_level > GLUSTER_DEBUG_MAX) {
-gconf->debug_level = GLUSTER_DEBUG_MAX;
+if (gconf->debug < 0) {
+gconf->debug = 0;
+} else if (gconf->debug > GLUSTER_DEBUG_MAX) {
+gconf->debug = GLUSTER_DEBUG_MAX;
}
-gconf->has_debug_level = true;
+gconf->has_debug = true;
gconf->logfile = qemu_opt_get_del(opts, GLUSTER_OPT_LOGFILE);
if (!gconf->logfile) {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index bcd3b9e..a569cfb 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2195,7 +2195,7 @@
#
# @server: gluster servers description
#
-# @debug-level: #optional libgfapi log level (default '4' which is Error)
+# @debug: #optional libgfapi log level (default '4' which is Error)
#
# @logfile: #optional libgfapi log file (default /dev/stderr) (Since 2.8)
#
@@ -2205,7 +2205,7 @@
'data': { 'volume': 'str',
'path': 'str',
'server': ['GlusterServer'],
-'*debug-level': 'int',
+'*debug': 'int',
'*logfile': 'str' } }
##
--
2.7.4
Re: [Qemu-devel] [PATCH v1 1/1] qemu-doc: update gluster protocol usage guide
On Wed, Nov 2, 2016 at 9:02 PM, Eric Blake wrote:
> On 11/02/2016 07:03 AM, Prasanna Kalever wrote:
>
>>> "debug":"N" does not match the schema; the parameter is named
>>> "debug-level", and it is an integer not a string. The parameter is
>>> optional; you could just omit it. But if you are going to include it,
>>> give a reasonable example like "debug-level":0.
>>
>> Eric,
>>
>>>From the code I can see
>> #define GLUSTER_OPT_DEBUG "debug"
>>
>> And even tried this, there is no option 'debug-level'.
>> And yes, I should admit it is 'int' not a 'string'
>
> Uggh. We were under so much pressure to try and get GlusterServer into
> 2.7 that we really botched it.
That's a great catch.
>
> The QMP definition is:
>
> { 'struct': 'BlockdevOptionsGluster',
> 'data': { 'volume': 'str',
> 'path': 'str',
> 'server': ['GlusterServer'],
> '*debug-level': 'int',
> '*logfile': 'str' } }
>
> and what's more, we've copied that string elsewhere for 2.8:
>
> { 'struct': 'BlockdevOptionsNfs',
> 'data': { 'server': 'NFSServer',
> 'path': 'str',
> '*user': 'int',
> '*group': 'int',
> '*tcp-syn-count': 'int',
> '*readahead-size': 'int',
> '*page-cache-size': 'int',
> '*debug-level': 'int' } }
>
> So either we need to fix the QMP to spell it 'debug' (to match the
> documentation you gave); or fix the docs to match the QMP and add some
> back-compat glue into the gluster code to manage both spellings rather
> than just the QMP spelling.
>
> Since blockdev-add is still experimental, I'm leaning towards changing
> the QMP for both gluster and NFS. But we need to get it done before
> 2.8. Looks like I have some patches to propose.
Agree!
Lets fix it in QMP definitions, since we have 'debug' option out in 2.7.
There could be people using this option now, changing the option may
break something out.
I have made the changes, will send them out.
Thanks for the suggest Eric.
--
Prasanna
>
> --
> Eric Blake eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
Re: [Qemu-devel] [PATCH v2] mttcg: Handle EXCP_ATOMIC exception
On 02/11/2016 17:40, Pranith Kumar wrote:
> The patch enables handling atomic code in the guest. This should be
> preferably done in cpu_handle_exception(), but the current assumptions
> regarding when we can execute atomic sections cause a deadlock.
>
> Signed-off-by: Pranith Kumar
> ---
> cpus.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/cpus.c b/cpus.c
> index 8f98060..299ce7e 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -1315,6 +1315,11 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
> if (r == EXCP_DEBUG) {
> cpu_handle_guest_debug(cpu);
> break;
> +} else if (r == EXCP_ATOMIC) {
> +qemu_mutex_unlock_iothread();
> +cpu_exec_step_atomic(cpu);
> +qemu_mutex_lock_iothread();
> +break;
> }
> } else if (cpu->stop) {
> if (cpu->unplug) {
> @@ -1385,6 +1390,10 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
> */
> g_assert(cpu->halted);
> break;
> +case EXCP_ATOMIC:
> +qemu_mutex_unlock_iothread();
> +cpu_exec_step_atomic(cpu);
> +qemu_mutex_lock_iothread();
> default:
> /* Ignore everything else? */
> break;
>
Alex, please pick up this patch yourself.
Paolo
Re: [Qemu-devel] [PATCH for-2.8?] rules.mak: speedup save-vars load-vars
On 02/11/2016 17:40, Daniel P. Berrange wrote: > Could we instead just do: > > Makefile.objs: > > util-obj-y = > include util/Makefile.objs > > util/Makefile.objs: > util-obj-y += util/osdep.o util/cutils.o util/unicode.o > > Yes, you now get the "burden" of adding a directory prefix onto the file > paths, but I don't think that's a big deal, as it is only a one-time > cost when you create a new file, or very rarely move files between dirs. I don't think the simplification would be that much. In particular, note that you would keep some of the complication to process *-cflags, *-libs and *-objs variables, and you would have additional complication to handle the prepending of ".." in Makefile.target. Paolo > Without the unnest-vars black magic I think it'd be simpler for mere > mortals to understand what is happening in the makefiles, lowering > the barrier to entry for new contributors to QEMU (and even existing > QEMU contributors who've not had the misfortune of debugging our > recursive make system yet) > > Regards, > Daniel >
[Qemu-devel] [PATCH v1 0/2] Add the generic ARM timer
These two patches and and connect the Generic ARM Timer. This includes support for dropping insecure writes. Alistair Francis (2): arm_generic_timer: Add the ARM Generic Timer xlnx-zynqmp: Connect the ARM Generic Timer hw/arm/xlnx-zynqmp.c | 13 +++ hw/timer/Makefile.objs | 1 + hw/timer/arm_generic_timer.c | 216 +++ include/hw/arm/xlnx-zynqmp.h | 2 + include/hw/timer/arm_generic_timer.h | 60 ++ 5 files changed, 292 insertions(+) create mode 100644 hw/timer/arm_generic_timer.c create mode 100644 include/hw/timer/arm_generic_timer.h -- 2.7.4
[Qemu-devel] [PATCH v1 2/2] xlnx-zynqmp: Connect the ARM Generic Timer
Signed-off-by: Alistair Francis
---
hw/arm/xlnx-zynqmp.c | 13 +
include/hw/arm/xlnx-zynqmp.h | 2 ++
2 files changed, 15 insertions(+)
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 0d86ba3..43c68c5 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -38,6 +38,8 @@
#define SATA_ADDR 0xFD0C
#define SATA_NUM_PORTS 2
+#define ARM_GEN_TIMER_ADDR 0xFF26
+
#define DP_ADDR 0xfd4a
#define DP_IRQ 113
@@ -172,6 +174,10 @@ static void xlnx_zynqmp_init(Object *obj)
qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
}
+object_initialize(&s->arm_gen_timer, sizeof(s->arm_gen_timer),
+ TYPE_ARM_GEN_TIMER);
+qdev_set_parent_bus(DEVICE(&s->arm_gen_timer), sysbus_get_default());
+
object_initialize(&s->dp, sizeof(s->dp), TYPE_XLNX_DP);
qdev_set_parent_bus(DEVICE(&s->dp), sysbus_get_default());
@@ -405,6 +411,13 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error
**errp)
g_free(bus_name);
}
+object_property_set_bool(OBJECT(&s->arm_gen_timer), true, "realized",
&err);
+if (err) {
+error_propagate(errp, err);
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->arm_gen_timer), 0, ARM_GEN_TIMER_ADDR);
+
object_property_set_bool(OBJECT(&s->dp), true, "realized", &err);
if (err) {
error_propagate(errp, err);
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index c2931bf..8deabb4 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -26,6 +26,7 @@
#include "hw/ide/ahci.h"
#include "hw/sd/sdhci.h"
#include "hw/ssi/xilinx_spips.h"
+#include "hw/timer/arm_generic_timer.h"
#include "hw/dma/xlnx_dpdma.h"
#include "hw/display/xlnx_dp.h"
@@ -83,6 +84,7 @@ typedef struct XlnxZynqMPState {
SysbusAHCIState sata;
SDHCIState sdhci[XLNX_ZYNQMP_NUM_SDHCI];
XilinxSPIPS spi[XLNX_ZYNQMP_NUM_SPIS];
+ARMGenTimer arm_gen_timer;
XlnxDPState dp;
XlnxDPDMAState dpdma;
--
2.7.4
Re: [Qemu-devel] [PATCH for-2.8?] rules.mak: speedup save-vars load-vars
On Wed, Nov 02, 2016 at 05:24:01PM +0100, Paolo Bonzini wrote: > Unnesting variables spends a lot of time parsing and executing foreach > and if functions. Because actually very few variables have to be > saved and restored, a good strategy is to remember what has to be done > in load-vars, and only iterate the right variables in load-vars. > For save-vars, unroll the foreach loop to provide another small > improvement. > > This speeds up a "noop" build from around 15.5 seconds on my laptop > to 11.7 (25% roughly). > > Signed-off-by: Paolo Bonzini > --- > I'm wondering if this would be acceptable for 2.8. > I also have sent patches to GNU make that save another > 20%, down to 9.8 seconds. > > rules.mak | 22 ++ > 1 file changed, 10 insertions(+), 12 deletions(-) I've no objection to your patch right now, but in general I wonder whether the complexity of unnest-vars is worth it. What is the intended benefit of using the unnest-vars approach, as opposed to explicitly including the sub-dir Makefiles and thus directly adding to the main variables without the recursive expansion step ? eg today we have: Makefile.objs: util-obj-y = util/ dummy := $(call unnest-vars,, util-obj-y) util/Makefile.objs: util-obj-y = osdep.o cutils.o unicode.o Could we instead just do: Makefile.objs: util-obj-y = include util/Makefile.objs util/Makefile.objs: util-obj-y += util/osdep.o util/cutils.o util/unicode.o Yes, you now get the "burden" of adding a directory prefix onto the file paths, but I don't think that's a big deal, as it is only a one-time cost when you create a new file, or very rarely move files between dirs. Without the unnest-vars black magic I think it'd be simpler for mere mortals to understand what is happening in the makefiles, lowering the barrier to entry for new contributors to QEMU (and even existing QEMU contributors who've not had the misfortune of debugging our recursive make system yet) Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://entangle-photo.org -o-http://search.cpan.org/~danberr/ :|
