Re: [Qemu-block] [Qemu-devel] [RFC v6 1/2] virtio: introduce `query-virtio' QMP command
On Tue, 19 Dec 2017 15:57:18 +0100
Markus Armbruster wrote:
> QAPI/QMP interface review only.
>
> You neglected to cc: the maintainers of qapi-schema.json, so I'm doing
> that for you.
>
Thank you so much. I screwed up with this patchset at least twice: with
list of maintainers and with my own e-mail.
> Jan Dakinevich writes:
>
> > The command is intended for gathering virtio information such as
> > status, feature bits, negotiation status. It is convenient and
> > useful for debug purpose.
> >
> > The commands returns generic virtio information for virtio such as
> > common feature names and status bits names and information for all
> > attached to current machine devices.
> >
> > To retrieve names of device-specific features `tell_feature_name'
> > callback in VirtioDeviceClass also was introduced.
> >
> > Cc: Denis V. Lunev
> > Signed-off-by: Jan Dakinevich
> [...]
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 1845795..51cd0f3 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -3200,3 +3200,73 @@
> > # Since: 2.11
> > ##
> > { 'command': 'watchdog-set-action', 'data' : {'action':
> > 'WatchdogAction'} } +
> > +##
> > +# @VirtioFeature:
> > +#
> > +# Virtio feature bit with negotiation status
> > +#
> > +# @name: name of feature bit
> > +#
> > +# @acked: negotiation status, `true' if guest acknowledged the
> > feature +#
> > +# Since: 2.12
> > +##
> > +{
> > +'struct': 'VirtioFeature',
> > +'data': {
> > +'name': 'str',
> > +'acked': 'bool'
> > +}
> > +}
> > +
> > +##
> > +# @VirtioInfo:
> > +#
> > +# Information about virtio device
> > +#
> > +# @qom-path: QOM path of the device
> > +#
> > +# @status: status bitmask
> > +#
> > +# @host-features: bitmask of features, exposed by device
> > +#
> > +# @guest-features: bitmask of features, acknowledged by guest
>
> Quoting my review of v4: "Unsigned integer interpreted as combination
> of well-known bit-valued symbols" is a fine C interface, but a pretty
> horrid QMP interface. What's wrong with doing a set the
> straightforward way as "array of enum"? As far as I can see, I
> didn't get a reply back then. You'll have to give one to get the
> patch accepted.
>
Most probably, I didn't understand you last time. Although, I'm still
far from correct understanding.
About 'array of enum', I could make the following description:
{
'enum': 'VirtioFeatureScsiBit',
'data': ['inout', 'hotplug', 'change', 'ta10_pi']
}
{
'struct': 'VirtioFeatureScsi',
'data' : {
'bit': 'VirtioFeatureScsiBit',
'acked': 'bool'
}
}
{
'enum': 'VirtioFeatureSerialBit',
'data': ['size', 'multiport', 'emerg-write']
}
{
'struct': 'VirtioFeatureSerial',
'data' : {
'bit': 'VirtioFeatureSerialBit',
'acked': 'bool'
}
}
{
'enum': 'VirtioFeatureCategory',
'data': ['scsi', 'serial']
}
{
'union': 'VirtioFeature',
'base': {
'category': 'VirtioFeatureCategory'
},
'discriminator': 'categoty',
'data': {
'scsi': 'VirtioFeatureScsi',
'serial': 'VirtioFeatureSerial'
}
}
{
'union': 'VirtioInfo',
'data': {
'qom-path': 'str',
'device-features': ['VirtioFeature']
...
}
}
But IMO, it only increases complexity without any benefit. To use this
description, at first it should be listed all features names in enums.
Then, these enums should be mapped into bit numbers and differently for
each virtio device. I expect something like this:
VirtioFeatureScsiBit bit = ...
switch (bit) {
case VIRTIO_FEATURE_SCSI_BIT_INOUT:
return VIRTIO_SCSI_F_INOUT;
case VIRTIO_FEATURE_SERIAL_BIT_HOTPLUG:
return VIRTIO_SCSI_F_HOTPLUG;
case VIRTIO_FEATURE_SERIAL_BIT_CHANGE:
return VIRTIO_SCSI_F_CHANGE;
case VIRTIO_FEATURE_SERIAL_BIT_T10_PI:
return VIRTIO_SCSI_F_T10_PI;
}
So, boilerplate for mapping between bit numbers and and theirs names
will not go away.
Furthermore, HMP command should produc
[Qemu-block] [Qemu-devel] [RFC v6 2/2] virtio: add `info virtio' HMP command
The command prints data from `query-virtio' QMP in human-readable
format.
Cc: Denis V. Lunev
Signed-off-by: Jan Dakinevich
---
hmp-commands-info.hx | 14 ++
hmp.c| 74
hmp.h| 1 +
3 files changed, 89 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 54c3e5e..292280a 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -868,6 +868,20 @@ enabled) memory in bytes.
ETEXI
STEXI
+@item info virtio
+@findex virtio
+Display guest and host fetures for all virtio devices.
+ETEXI
+
+{
+.name = "virtio",
+.args_type = "path:s?",
+.params = "[path]",
+.help = "show virtio info",
+.cmd= hmp_info_virtio,
+},
+
+STEXI
@end table
ETEXI
diff --git a/hmp.c b/hmp.c
index 35a7041..786782b 100644
--- a/hmp.c
+++ b/hmp.c
@@ -43,6 +43,7 @@
#include "hw/intc/intc.h"
#include "migration/snapshot.h"
#include "migration/misc.h"
+#include "hw/virtio/virtio.h"
#ifdef CONFIG_SPICE
#include
@@ -2918,3 +2919,76 @@ void hmp_info_memory_size_summary(Monitor *mon, const
QDict *qdict)
}
hmp_handle_error(mon, &err);
}
+
+#define HMP_INFO_VIRTIO_INDENT 2
+#define HMP_INFO_VIRTIO_FEATURE 40
+
+static void hmp_info_virtio_print(Monitor *mon, VirtioInfo *info)
+{
+Object *obj = object_resolve_path(info->qom_path, NULL);
+char *path = qdev_get_dev_path(DEVICE(obj));
+VirtioFeatureList *feat;
+strList *status;
+
+monitor_printf(mon, "%s at %s\n", object_get_typename(obj), path);
+g_free(path);
+
+monitor_printf(mon, "%*sQOM path: %s\n",
+ HMP_INFO_VIRTIO_INDENT, "", info->qom_path);
+
+/* device status */
+monitor_printf(mon, "%*sstatus: 0x%02"PRIx8"\n",
+ HMP_INFO_VIRTIO_INDENT, "", info->status);
+for (status = info->status_names; status; status = status->next) {
+monitor_printf(mon, "%*s%s\n",
+ HMP_INFO_VIRTIO_INDENT * 2, "", status->value);
+}
+
+
+/* host and guest feature */
+monitor_printf(mon, "%*shost features: 0x%016"PRIx64"\n",
+ HMP_INFO_VIRTIO_INDENT, "", info->host_features);
+monitor_printf(mon, "%*sguest features: 0x%016"PRIx64"\n",
+ HMP_INFO_VIRTIO_INDENT, "", info->guest_features);
+
+/* common features */
+monitor_printf(mon, "%*scommon features:%s\n",
+ HMP_INFO_VIRTIO_INDENT, "",
+ info->common_features_names ? "" : " (none)");
+for (feat = info->common_features_names; feat; feat = feat->next) {
+monitor_printf(mon, "%*s%-*s%s\n",
+ HMP_INFO_VIRTIO_INDENT * 2, "",
+ HMP_INFO_VIRTIO_FEATURE, feat->value->name,
+ feat->value->acked ? " acked" : "");
+}
+
+/* device features */
+monitor_printf(mon, "%*sdevice features:%s\n",
+ HMP_INFO_VIRTIO_INDENT, "",
+ info->device_features_names ? "" : " (none)");
+for (feat = info->device_features_names; feat; feat = feat->next) {
+monitor_printf(mon, "%*s%-*s%s\n",
+ HMP_INFO_VIRTIO_INDENT * 2, "",
+ HMP_INFO_VIRTIO_FEATURE, feat->value->name,
+ feat->value->acked ? " acked" : "");
+}
+}
+
+void hmp_info_virtio(Monitor *mon, const QDict *qdict)
+{
+const char *path = qdict_get_try_str(qdict, "path");
+Error *err = NULL;
+VirtioInfoList *head, *info;
+
+
+head = qmp_query_virtio(!!path, path, &err);
+if (err) {
+return;
+}
+
+for (info = head; info; info = info->next) {
+hmp_info_virtio_print(mon, info->value);
+}
+
+qapi_free_VirtioInfoList(head);
+}
diff --git a/hmp.h b/hmp.h
index a6f56b1..156293e 100644
--- a/hmp.h
+++ b/hmp.h
@@ -147,5 +147,6 @@ void hmp_info_ramblock(Monitor *mon, const QDict *qdict);
void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict);
void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
+void hmp_info_virtio(Monitor *mon, const QDict *qdict);
#endif
--
2.1.4
[Qemu-block] [Qemu-devel] [RFC v6 1/2] virtio: introduce `query-virtio' QMP command
The command is intended for gathering virtio information such as status,
feature bits, negotiation status. It is convenient and useful for debug
purpose.
The commands returns generic virtio information for virtio such as
common feature names and status bits names and information for all
attached to current machine devices.
To retrieve names of device-specific features `tell_feature_name'
callback in VirtioDeviceClass also was introduced.
Cc: Denis V. Lunev
Signed-off-by: Jan Dakinevich
---
hw/block/virtio-blk.c | 20 +++
hw/char/virtio-serial-bus.c | 14 +
hw/display/virtio-gpu.c | 12
hw/net/virtio-net.c | 34 +++
hw/scsi/virtio-scsi.c | 15 +
hw/virtio/Makefile.objs | 3 +
hw/virtio/virtio-balloon.c | 14 +
hw/virtio/virtio-qmp.c | 134
hw/virtio/virtio-stub.c | 9 +++
hw/virtio/virtio.c | 41 ++
include/hw/virtio/virtio.h | 6 ++
qapi-schema.json| 70 +++
12 files changed, 372 insertions(+)
create mode 100644 hw/virtio/virtio-qmp.c
create mode 100644 hw/virtio/virtio-stub.c
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 05d1440..2ffd949 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1017,6 +1017,25 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static const char *virtio_blk_tell_feature_name(VirtIODevice *vdev,
+unsigned fbit)
+{
+#define FBIT(fbit) case fbit: return #fbit
+switch (fbit) {
+FBIT(VIRTIO_BLK_F_BARRIER);
+FBIT(VIRTIO_BLK_F_SIZE_MAX);
+FBIT(VIRTIO_BLK_F_SEG_MAX);
+FBIT(VIRTIO_BLK_F_RO);
+FBIT(VIRTIO_BLK_F_BLK_SIZE);
+FBIT(VIRTIO_BLK_F_SCSI);
+FBIT(VIRTIO_BLK_F_TOPOLOGY);
+FBIT(VIRTIO_BLK_F_FLUSH);
+FBIT(VIRTIO_BLK_F_MQ);
+}
+#undef FBIT
+return NULL;
+}
+
static void virtio_blk_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1030,6 +1049,7 @@ static void virtio_blk_class_init(ObjectClass *klass,
void *data)
vdc->get_config = virtio_blk_update_config;
vdc->set_config = virtio_blk_set_config;
vdc->get_features = virtio_blk_get_features;
+vdc->tell_feature_name = virtio_blk_tell_feature_name;
vdc->set_status = virtio_blk_set_status;
vdc->reset = virtio_blk_reset;
vdc->save = virtio_blk_save_device;
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 9470bd7..1d4678b 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1156,6 +1156,19 @@ static Property virtio_serial_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static const char *virtio_serial_tell_feature_name(VirtIODevice *vdev,
+ unsigned fbit)
+{
+#define FBIT(fbit) case fbit: return #fbit
+switch (fbit) {
+FBIT(VIRTIO_CONSOLE_F_SIZE);
+FBIT(VIRTIO_CONSOLE_F_MULTIPORT);
+FBIT(VIRTIO_CONSOLE_F_EMERG_WRITE);
+};
+#undef FBIT
+return NULL;
+}
+
static void virtio_serial_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1170,6 +1183,7 @@ static void virtio_serial_class_init(ObjectClass *klass,
void *data)
vdc->realize = virtio_serial_device_realize;
vdc->unrealize = virtio_serial_device_unrealize;
vdc->get_features = get_features;
+vdc->tell_feature_name = virtio_serial_tell_feature_name;
vdc->get_config = get_config;
vdc->set_config = set_config;
vdc->set_status = set_status;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 274e365..1906b68 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1317,6 +1317,17 @@ static Property virtio_gpu_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static const char *virtio_gpu_tell_feature_name(VirtIODevice *vdev,
+unsigned fbit)
+{
+#define FBIT(fbit) case fbit: return #fbit
+switch (fbit) {
+FBIT(VIRTIO_GPU_F_VIRGL);
+};
+#undef FBIT
+return NULL;
+}
+
static void virtio_gpu_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1327,6 +1338,7 @@ static void virtio_gpu_class_init(ObjectClass *klass,
void *data)
vdc->get_config = virtio_gpu_get_config;
vdc->set_config = virtio_gpu_set_config;
vdc->get_features = virtio_gpu_get_features;
+vdc->tell_feature_name = virtio_gpu_tell_feature_name;
vdc->set_features = virtio_gpu_set_features;
vdc->reset = virtio_gpu_reset;
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 38674b0..0fbd055 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -2163,6 +2163,39 @@ static Property virtio_net_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+s
[Qemu-block] [Qemu-devel] [RFC v6 0/2] virtio: introduce `info virtio' hmp command
From: Jan Dakinevich After some discussion, I am going to suggest reworked QMP/HMP for gathering virtio info. It would provide the following monitor output. (qemu) info virtio virtio-blk-device at :00:02.0 QOM path: /machine/peripheral-anon/device[0]/virtio-backend status: 0x0f VIRTIO_CONFIG_S_ACKNOWLEDGE VIRTIO_CONFIG_S_DRIVER VIRTIO_CONFIG_S_DRIVER_OK VIRTIO_CONFIG_S_FEATURES_OK host features: 0x000179000e54 guest features: 0x00013e54 common features: VIRTIO_F_NOTIFY_ON_EMPTY VIRTIO_F_ANY_LAYOUT VIRTIO_RING_F_INDIRECT_DESC acked VIRTIO_RING_F_EVENT_IDX acked VIRTIO_F_BAD_FEATURE VIRTIO_F_VERSION_1 acked device features: VIRTIO_BLK_F_SEG_MAX acked VIRTIO_BLK_F_BLK_SIZEacked VIRTIO_BLK_F_FLUSH acked VIRTIO_BLK_F_TOPOLOGYacked v5: http://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg05667.html v4: http://lists.nongnu.org/archive/html/qemu-devel/2017-10/msg00393.html v3: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07565.html v2: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07527.html v1: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html Jan Dakinevich (2): virtio: introduce `query-virtio' QMP command virtio: add `info virtio' HMP command hmp-commands-info.hx| 14 + hmp.c | 74 hmp.h | 1 + hw/block/virtio-blk.c | 20 +++ hw/char/virtio-serial-bus.c | 14 + hw/display/virtio-gpu.c | 12 hw/net/virtio-net.c | 34 +++ hw/scsi/virtio-scsi.c | 15 + hw/virtio/Makefile.objs | 3 + hw/virtio/virtio-balloon.c | 14 + hw/virtio/virtio-qmp.c | 134 hw/virtio/virtio-stub.c | 9 +++ hw/virtio/virtio.c | 41 ++ include/hw/virtio/virtio.h | 6 ++ qapi-schema.json| 70 +++ 15 files changed, 461 insertions(+) create mode 100644 hw/virtio/virtio-qmp.c create mode 100644 hw/virtio/virtio-stub.c -- 2.1.4
Re: [Qemu-block] [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command
On 10/04/2017 07:00 PM, Eric Blake wrote:
> On 10/04/2017 09:26 AM, Jan Dakinevich wrote:
>
>>>>>> +{
>>>>>> +'struct': 'VirtioInfo',
>>>>>> +'data': {
>>>>>> +'feature-names': ['VirtioInfoBit'],
>>>>>
>>>>> Why is feature-names listed at two different nestings of the return value?
>>>>>
>>>>
>>>> These are different feature names. First names are common and predefined
>>>> for all devices. Second names are device-specific.
>>>
>>> If you can turn these into enums (union'd enums?) then you might
>>> be able to get rid of a lot of your array filling/naming conversion
>>> boilerplate. (Not sure if it's worth it, but it's worth looking).
>>>
>>
>> I would be happy to drop this boilerplate, but how enum could help here?
>> To respond my requirement it should be something like set, not enum.
>> Even so, having set, I would have been needed to declare mapping between
>> names in set type and bit numbers within feature bitmask.
>
> Instead of returning a bitmask ("mask":123) as well as an array naming
> those bits
> ([{"bit":1,"name":"bit1"},{"bit":2","name":"bit2"},{"bit":4,"name":"bit4},...]),
> you could omit the bit numbers and just return an array of named bits
> (["bit1", "bit2", "bit4"]). An enum lets you declare up front what
> named bits are supported (and code can introspect when new named bits
> are supported in newer qemu).
>
But how can I declare in this notation that name "bit1" is owned by bit
#1, name "bit2" is owned by bit #2, name "bit4" is owned by bit #4, and
all other bits has no names.
> Perhaps it's easier to first take a step back, and show what the desired
> output might be like, and then we can figure out how to represent that
> output in QAPI.
>
Yeah... I expect the following HMP output:
(qmue) info virtio
virtio-blk-device at :00:07.0
status: 0x07 acknowledge,driver,driver_ok
host features:0x000179000e54
guest features: 0x3e54
common features:
notify_on_empty
any_layout
indirect_desc acked
event_idx acked
version_1
specific features:
seg_max acked
blk_size acked
flush acked
topology acked
virtio-serial-device at :00:08.0
status: 0x07 acknowledge,driver,driver_ok
host features:0x00017906
guest features: 0x3002
common features:
notify_on_empty
any_layout
indirect_desc acked
event_idx acked
version_1
specific features:
multiport acked
emerg_write
--
Best regards
Jan Dakinevich
Re: [Qemu-block] [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command
On 10/03/2017 07:29 PM, Dr. David Alan Gilbert wrote:
> * Jan Dakinevich (jan.dakinev...@virtuozzo.com) wrote:
>>
>>
>> On 10/03/2017 05:02 PM, Eric Blake wrote:
>>> On 10/03/2017 07:47 AM, Jan Dakinevich wrote:
>>>> The command is intended for gathering virtio information such as status,
>>>> feature bits, negotiation status. It is convenient and useful for debug
>>>> purpose.
>>>>
>>>> The commands returns generic virtio information for virtio such as
>>>> common feature names and status bits names and information for all
>>>> attached to current machine devices.
>>>>
>>>> To retrieve names of device-specific features `get_feature_name'
>>>> callback in VirtioDeviceClass also was introduced.
>>>>
>>>> Cc: Denis V. Lunev
>>>> Signed-off-by: Jan Dakinevich
>>>> ---
>>>> hw/block/virtio-blk.c | 21 +
>>>> hw/char/virtio-serial-bus.c | 15 +++
>>>> hw/display/virtio-gpu.c | 13 ++
>>>> hw/net/virtio-net.c | 35 +++
>>>> hw/scsi/virtio-scsi.c | 16 +++
>>>> hw/virtio/Makefile.objs | 2 +
>>>> hw/virtio/virtio-balloon.c | 15 +++
>>>> hw/virtio/virtio-stub.c | 9
>>>> hw/virtio/virtio.c | 101
>>>>
>>>> include/hw/virtio/virtio.h | 2 +
>>>> qapi-schema.json| 1 +
>>>> qapi/virtio.json| 94
>>>> +
>>>> 12 files changed, 324 insertions(+)
>>>> create mode 100644 hw/virtio/virtio-stub.c
>>>> create mode 100644 qapi/virtio.json
>>>
>>> This creates a new .json file, but does not touch MAINTAINERS. Our idea
>>> in splitting the .json files was to make it easier for each sub-file
>>> that needs a specific maintainer in addition to the overall *.json line
>>> for QAPI maintainers, so this may deserve a MAINTAINERS entry.
>>>
>>
>> Ok.
>>
>>>> +++ b/qapi/virtio.json
>>>> @@ -0,0 +1,94 @@
>>>> +# -*- Mode: Python -*-
>>>> +#
>>>> +
>>>> +##
>>>> +# = Virtio devices
>>>> +##
>>>> +
>>>> +{ 'include': 'common.json' }
>>>> +
>>>> +##
>>>> +# @VirtioInfoBit:
>>>> +#
>>>> +# Named virtio bit
>>>> +#
>>>> +# @bit: bit number
>>>> +#
>>>> +# @name: bit name
>>>> +#
>>>> +# Since: 2.11.0
>>>> +#
>>>> +##
>>>> +{
>>>> +'struct': 'VirtioInfoBit',
>>>> +'data': {
>>>> +'bit': 'uint64',
>>>
>>> Why is this a 64-bit value? Are the values 0-63, or are they 1, 2, 4, 8,
>>> ...? The documentation on 'bit number' is rather sparse.
>>
>> I would prefer `uint' here, but I don't see generic unsigned type (may
>> be, I am mistaken). I could use uint8 here, though.
>>
>>>
>>>> +'name': 'str'
>>>
>>> Wouldn't an enum type be better than an open-ended string?
>>>
>>
>> Bit names are not known here, they are obtained from virtio device
>> implementations.
>>
>>>> +}
>>>> +}
>>>> +
>>>> +##
>>>> +# @VirtioInfoDevice:
>>>> +#
>>>> +# Information about specific virtio device
>>>> +#
>>>> +# @qom_path: QOM path of the device
>>>
>>> Please make this 'qom-path' - new interfaces should prefer '-' over '_'.
>>
>> Ok.
>>
>>>> +#
>>>> +# @feature-names: names of device-specific features
>>>> +#
>>>> +# @host-features: bitmask of features, provided by devices
>>>> +#
>>>> +# @guest-features: bitmask of features, acknowledged by guest
>>>> +#
>>>> +# @status: virtio device status bitmask
>>>> +#
>>>> +# Since: 2.11.0
>>>> +#
>>>> +##
>>>> +{
>>>> +'struct': 'VirtioInfoDevice',
>>>> +'data': {
>>>> +'qom_path': 'str',
>>>> +'feature-names
Re: [Qemu-block] [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command
On 10/03/2017 05:02 PM, Eric Blake wrote:
> On 10/03/2017 07:47 AM, Jan Dakinevich wrote:
>> The command is intended for gathering virtio information such as status,
>> feature bits, negotiation status. It is convenient and useful for debug
>> purpose.
>>
>> The commands returns generic virtio information for virtio such as
>> common feature names and status bits names and information for all
>> attached to current machine devices.
>>
>> To retrieve names of device-specific features `get_feature_name'
>> callback in VirtioDeviceClass also was introduced.
>>
>> Cc: Denis V. Lunev
>> Signed-off-by: Jan Dakinevich
>> ---
>> hw/block/virtio-blk.c | 21 +
>> hw/char/virtio-serial-bus.c | 15 +++
>> hw/display/virtio-gpu.c | 13 ++
>> hw/net/virtio-net.c | 35 +++
>> hw/scsi/virtio-scsi.c | 16 +++
>> hw/virtio/Makefile.objs | 2 +
>> hw/virtio/virtio-balloon.c | 15 +++
>> hw/virtio/virtio-stub.c | 9
>> hw/virtio/virtio.c | 101
>>
>> include/hw/virtio/virtio.h | 2 +
>> qapi-schema.json| 1 +
>> qapi/virtio.json| 94 +
>> 12 files changed, 324 insertions(+)
>> create mode 100644 hw/virtio/virtio-stub.c
>> create mode 100644 qapi/virtio.json
>
> This creates a new .json file, but does not touch MAINTAINERS. Our idea
> in splitting the .json files was to make it easier for each sub-file
> that needs a specific maintainer in addition to the overall *.json line
> for QAPI maintainers, so this may deserve a MAINTAINERS entry.
>
Ok.
>> +++ b/qapi/virtio.json
>> @@ -0,0 +1,94 @@
>> +# -*- Mode: Python -*-
>> +#
>> +
>> +##
>> +# = Virtio devices
>> +##
>> +
>> +{ 'include': 'common.json' }
>> +
>> +##
>> +# @VirtioInfoBit:
>> +#
>> +# Named virtio bit
>> +#
>> +# @bit: bit number
>> +#
>> +# @name: bit name
>> +#
>> +# Since: 2.11.0
>> +#
>> +##
>> +{
>> +'struct': 'VirtioInfoBit',
>> +'data': {
>> +'bit': 'uint64',
>
> Why is this a 64-bit value? Are the values 0-63, or are they 1, 2, 4, 8,
> ...? The documentation on 'bit number' is rather sparse.
I would prefer `uint' here, but I don't see generic unsigned type (may
be, I am mistaken). I could use uint8 here, though.
>
>> +'name': 'str'
>
> Wouldn't an enum type be better than an open-ended string?
>
Bit names are not known here, they are obtained from virtio device
implementations.
>> +}
>> +}
>> +
>> +##
>> +# @VirtioInfoDevice:
>> +#
>> +# Information about specific virtio device
>> +#
>> +# @qom_path: QOM path of the device
>
> Please make this 'qom-path' - new interfaces should prefer '-' over '_'.
Ok.
>> +#
>> +# @feature-names: names of device-specific features
>> +#
>> +# @host-features: bitmask of features, provided by devices
>> +#
>> +# @guest-features: bitmask of features, acknowledged by guest
>> +#
>> +# @status: virtio device status bitmask
>> +#
>> +# Since: 2.11.0
>> +#
>> +##
>> +{
>> +'struct': 'VirtioInfoDevice',
>> +'data': {
>> +'qom_path': 'str',
>> +'feature-names': ['VirtioInfoBit'],
>> +'host-features': 'uint64',
>> +'guest-features': 'uint64',
>> +'status': 'uint64'
>
> I'm wondering if this is the best representation (where the caller has
> to parse the integer and then lookup in feature-names what each bit of
> the integer represents). But I'm not sure I have anything better off
> the top of my head.
>
Consider it as way to tell caller about names of supported features.
>> +}
>> +}
>> +
>> +##
>> +# @VirtioInfo:
>> +#
>> +# Information about virtio devices
>> +#
>> +# @feature-names: names of common virtio features
>> +#
>> +# @status-names: names of bits which represents virtio device status
>> +#
>> +# @devices: list of per-device virtio information
>> +#
>> +# Since: 2.11.0
>> +#
>> +##
>> +{
>> +'struct': 'VirtioInfo',
>> +'data': {
>> +'feature-names': ['VirtioInfoBit'],
>
> Why is feature-names listed at two different nestings of the return value?
>
These are different feature names. First names are common and predefined
for all devices. Second names are device-specific.
>> +'status-names': ['VirtioInfoBit'],
>> +'devices': ['VirtioInfoDevice']
>> +}
>> +}
>> +
>> +
>> +##
>> +# @query-virtio:
>> +#
>> +# Returns generic and per-device virtio information
>> +#
>> +# Since: 2.11.0
>> +#
>> +##
>> +{
>> +'command': 'query-virtio',
>> +'returns': 'VirtioInfo'
>> +}
>>
>
--
Best regards
Jan Dakinevich
[Qemu-block] [Qemu-devel] [PATCH v4 1/2] virtio: introduce `query-virtio' QMP command
The command is intended for gathering virtio information such as status,
feature bits, negotiation status. It is convenient and useful for debug
purpose.
The commands returns generic virtio information for virtio such as
common feature names and status bits names and information for all
attached to current machine devices.
To retrieve names of device-specific features `get_feature_name'
callback in VirtioDeviceClass also was introduced.
Cc: Denis V. Lunev
Signed-off-by: Jan Dakinevich
---
hw/block/virtio-blk.c | 21 +
hw/char/virtio-serial-bus.c | 15 +++
hw/display/virtio-gpu.c | 13 ++
hw/net/virtio-net.c | 35 +++
hw/scsi/virtio-scsi.c | 16 +++
hw/virtio/Makefile.objs | 2 +
hw/virtio/virtio-balloon.c | 15 +++
hw/virtio/virtio-stub.c | 9
hw/virtio/virtio.c | 101
include/hw/virtio/virtio.h | 2 +
qapi-schema.json| 1 +
qapi/virtio.json| 94 +
12 files changed, 324 insertions(+)
create mode 100644 hw/virtio/virtio-stub.c
create mode 100644 qapi/virtio.json
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 05d1440..c77f651 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1017,6 +1017,26 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static const char *virtio_blk_get_feature_name(VirtIODevice *vdev,
+ unsigned feature)
+{
+static const char *names[] = {
+[VIRTIO_BLK_F_BARRIER] = "barrier",
+[VIRTIO_BLK_F_SIZE_MAX] = "size_max",
+[VIRTIO_BLK_F_SEG_MAX] = "seg_max",
+[VIRTIO_BLK_F_RO] = "ro",
+[VIRTIO_BLK_F_BLK_SIZE] = "blk_size",
+[VIRTIO_BLK_F_SCSI] = "scsi",
+[VIRTIO_BLK_F_TOPOLOGY] = "topology",
+[VIRTIO_BLK_F_FLUSH] = "flush",
+[VIRTIO_BLK_F_MQ] = "mq",
+};
+if (feature >= ARRAY_SIZE(names)) {
+return NULL;
+}
+return names[feature];
+}
+
static void virtio_blk_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1030,6 +1050,7 @@ static void virtio_blk_class_init(ObjectClass *klass,
void *data)
vdc->get_config = virtio_blk_update_config;
vdc->set_config = virtio_blk_set_config;
vdc->get_features = virtio_blk_get_features;
+vdc->get_feature_name = virtio_blk_get_feature_name;
vdc->set_status = virtio_blk_set_status;
vdc->reset = virtio_blk_reset;
vdc->save = virtio_blk_save_device;
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 9470bd7..41f4466 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1156,6 +1156,20 @@ static Property virtio_serial_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static const char *virtio_serial_get_feature_name(VirtIODevice *vdev,
+ unsigned feature)
+{
+static const char *names[] = {
+[VIRTIO_CONSOLE_F_SIZE] = "size",
+[VIRTIO_CONSOLE_F_MULTIPORT] = "multiport",
+[VIRTIO_CONSOLE_F_EMERG_WRITE] = "emerg_write",
+};
+if (feature >= ARRAY_SIZE(names)) {
+return NULL;
+}
+return names[feature];
+}
+
static void virtio_serial_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1170,6 +1184,7 @@ static void virtio_serial_class_init(ObjectClass *klass,
void *data)
vdc->realize = virtio_serial_device_realize;
vdc->unrealize = virtio_serial_device_unrealize;
vdc->get_features = get_features;
+vdc->get_feature_name = virtio_serial_get_feature_name;
vdc->get_config = get_config;
vdc->set_config = set_config;
vdc->set_status = set_status;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 3a8f1e1..49aa214 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1307,6 +1307,18 @@ static Property virtio_gpu_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static const char *virtio_gpu_get_feature_name(VirtIODevice *vdev,
+ unsigned feature)
+{
+static const char *names[] = {
+[VIRTIO_GPU_F_VIRGL] = "virgl",
+};
+if (feature >= ARRAY_SIZE(names)) {
+return NULL;
+}
+return names[feature];
+}
+
static void virtio_gpu_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1317,6 +1329,7 @@ static void virtio_gpu_class_init(ObjectClass *klass,
void *data)
vdc->get_config = virtio_gpu_get_config;
vdc->set_config = virtio_gpu_set_config;
vdc->get_features = virtio_gpu_get_features;
Re: [Qemu-block] [Qemu-devel] [PATCH v2] virtio: introduce `info virtio' hmp command
> OK but if it's useful as an hmp command, why not as a qmp command?
>
The command is designed for debugging and produces quite sightly output.
For respective qmp command most of `info virtio' output would excessive
and unneccesary.
On 09/27/2017 10:43 PM, Michael S. Tsirkin wrote:
On Wed, Sep 27, 2017 at 06:09:42PM +0300, Jan Dakinevich wrote:
The command is intended for exposing device specific virtio feature bits
and their negotiation status. It is convenient and useful for debug
purpose.
Names of features are taken from a devices via get_feature_name() within
VirtioDeviceClass. If certain device doesn't implement it, the command
will print only hexadecimal value of feature mask.
Cc: Denis V. Lunev
Signed-off-by: Jan Dakinevich
OK but if it's useful as an hmp command, why not as a qmp command?
---
v2:
* Moved hmp_info_virtio and stuff to hmp.c to avoid build error
* Added printing of device status
* Listed common virtio features
v1: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html
---
hmp-commands-info.hx| 14 +
hmp.c | 148
hmp.h | 1 +
hw/block/virtio-blk.c | 20 ++
hw/char/virtio-serial-bus.c | 15 +
hw/display/virtio-gpu.c | 12
hw/net/virtio-net.c | 34 ++
hw/scsi/virtio-scsi.c | 15 +
hw/virtio/virtio-balloon.c | 15 +
hw/virtio/virtio-bus.c | 1 +
include/hw/virtio/virtio.h | 2 +
monitor.c | 1 +
12 files changed, 278 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 4f1ece9..2550027 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -868,6 +868,20 @@ ETEXI
},
STEXI
+@item info virtio
+@findex virtio
+Display guest and host fetures for all virtio devices.
+ETEXI
+
+{
+.name = "virtio",
+.args_type = "",
+.params = "",
+.help = "show virtio info",
+.cmd= hmp_info_virtio,
+},
+
+STEXI
@end table
ETEXI
diff --git a/hmp.c b/hmp.c
index ace729d..009d808 100644
--- a/hmp.c
+++ b/hmp.c
@@ -43,6 +43,7 @@
#include "hw/intc/intc.h"
#include "migration/snapshot.h"
#include "migration/misc.h"
+#include "hw/virtio/virtio.h"
#ifdef CONFIG_SPICE
#include
@@ -2894,3 +2895,150 @@ void hmp_info_memory_size_summary(Monitor *mon, const
QDict *qdict)
}
hmp_handle_error(mon, &err);
}
+
+#define HMP_INFO_VIRTIO_INDENT 2
+#define HMP_INFO_VIRTIO_FIELD 32
+
+static void hmp_info_virtio_print_status(Monitor *mon, VirtIODevice *vdev)
+{
+uint8_t status[] = {
+VIRTIO_CONFIG_S_ACKNOWLEDGE,
+VIRTIO_CONFIG_S_DRIVER,
+VIRTIO_CONFIG_S_DRIVER_OK,
+VIRTIO_CONFIG_S_FEATURES_OK,
+VIRTIO_CONFIG_S_NEEDS_RESET,
+VIRTIO_CONFIG_S_FAILED,
+};
+const char *names[] = {
+"acknowledge",
+"driver",
+"driver_ok",
+"features_ok",
+"needs_reset"
+"failed",
+};
+const char *comma = "";
+int idx;
+
+monitor_printf(mon, "%*sstatus: 0x%02"PRIx8" ",
+ HMP_INFO_VIRTIO_INDENT, "", vdev->status);
+
+for (idx = 0; idx < ARRAY_SIZE(status); idx++) {
+if (!(vdev->status & status[idx])) {
+continue;
+}
+monitor_printf(mon, "%s%s", comma, names[idx]);
+if (names[idx]) {
+comma = ",";
+}
+}
+monitor_printf(mon, "\n");
+}
+
+static void hmp_info_virtio_print_common_features(Monitor *mon,
+ VirtIODevice *vdev)
+{
+uint64_t features[] = {
+VIRTIO_RING_F_INDIRECT_DESC,
+VIRTIO_RING_F_EVENT_IDX,
+VIRTIO_F_NOTIFY_ON_EMPTY,
+VIRTIO_F_ANY_LAYOUT,
+VIRTIO_F_IOMMU_PLATFORM,
+VIRTIO_F_VERSION_1,
+};
+const char *names[] = {
+"indirect_desc",
+"event_idx",
+"notify_on_empty",
+"any_layout",
+"iommu_platform",
+"version_1",
+};
+int idx;
+
+monitor_printf(mon, "%*scommon features:\n", HMP_INFO_VIRTIO_INDENT, "");
+
+for (idx = 0; idx < ARRAY_SIZE(features); idx++) {
+const char *ack = vdev->guest_features & features[idx] ? "acked" : "";
+
+if (!(vdev->host_features & features[idx])) {
+continue;
+}
+monitor_printf(mon, "%*s%*s%*s\n", HMP_INFO_VIRTIO_INDENT, "",
+ HMP_INFO_VIRTIO_FIELD, names[idx],
+ HMP_IN
[Qemu-block] [Qemu-devel] [PATCH v3] virtio: introduce `info virtio' hmp command
The command is intended for exposing device specific virtio feature bits
and their negotiation status. It is convenient and useful for debug
purpose.
Names of features are taken from a devices via get_feature_name() within
VirtioDeviceClass. If certain device doesn't implement it, the command
will print only hexadecimal value of feature mask.
Cc: Denis V. Lunev
Signed-off-by: Jan Dakinevich
---
v3:
* Avoid signed int
* Use virtio_vdev_has_feature()/virtio_host_has_feature()
v2: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07527.html
v1: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html
---
hmp-commands-info.hx| 14 +
hmp.c | 149
hmp.h | 1 +
hw/block/virtio-blk.c | 21 +++
hw/char/virtio-serial-bus.c | 15 +
hw/display/virtio-gpu.c | 13
hw/net/virtio-net.c | 35 +++
hw/scsi/virtio-scsi.c | 16 +
hw/virtio/virtio-balloon.c | 15 +
hw/virtio/virtio-bus.c | 1 +
include/hw/virtio/virtio.h | 2 +
monitor.c | 1 +
12 files changed, 283 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 4f1ece9..2550027 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -868,6 +868,20 @@ ETEXI
},
STEXI
+@item info virtio
+@findex virtio
+Display guest and host fetures for all virtio devices.
+ETEXI
+
+{
+.name = "virtio",
+.args_type = "",
+.params = "",
+.help = "show virtio info",
+.cmd= hmp_info_virtio,
+},
+
+STEXI
@end table
ETEXI
diff --git a/hmp.c b/hmp.c
index ace729d..f231395 100644
--- a/hmp.c
+++ b/hmp.c
@@ -43,6 +43,7 @@
#include "hw/intc/intc.h"
#include "migration/snapshot.h"
#include "migration/misc.h"
+#include "hw/virtio/virtio.h"
#ifdef CONFIG_SPICE
#include
@@ -2894,3 +2895,151 @@ void hmp_info_memory_size_summary(Monitor *mon, const
QDict *qdict)
}
hmp_handle_error(mon, &err);
}
+
+#define HMP_INFO_VIRTIO_INDENT 2
+#define HMP_INFO_VIRTIO_FIELD 32
+
+static void hmp_info_virtio_print_status(Monitor *mon, VirtIODevice *vdev)
+{
+uint8_t status[] = {
+VIRTIO_CONFIG_S_ACKNOWLEDGE,
+VIRTIO_CONFIG_S_DRIVER,
+VIRTIO_CONFIG_S_DRIVER_OK,
+VIRTIO_CONFIG_S_FEATURES_OK,
+VIRTIO_CONFIG_S_NEEDS_RESET,
+VIRTIO_CONFIG_S_FAILED,
+};
+const char *names[] = {
+"acknowledge",
+"driver",
+"driver_ok",
+"features_ok",
+"needs_reset"
+"failed",
+};
+const char *comma = "";
+unsigned idx;
+
+monitor_printf(mon, "%*sstatus: 0x%02"PRIx8" ",
+ HMP_INFO_VIRTIO_INDENT, "", vdev->status);
+
+for (idx = 0; idx < ARRAY_SIZE(status); idx++) {
+if (!(vdev->status & status[idx])) {
+continue;
+}
+monitor_printf(mon, "%s%s", comma, names[idx]);
+if (names[idx]) {
+comma = ",";
+}
+}
+monitor_printf(mon, "\n");
+}
+
+static void hmp_info_virtio_print_common_features(Monitor *mon,
+ VirtIODevice *vdev)
+{
+uint64_t features[] = {
+VIRTIO_RING_F_INDIRECT_DESC,
+VIRTIO_RING_F_EVENT_IDX,
+VIRTIO_F_NOTIFY_ON_EMPTY,
+VIRTIO_F_ANY_LAYOUT,
+VIRTIO_F_IOMMU_PLATFORM,
+VIRTIO_F_VERSION_1,
+};
+const char *names[] = {
+"indirect_desc",
+"event_idx",
+"notify_on_empty",
+"any_layout",
+"iommu_platform",
+"version_1",
+};
+unsigned idx;
+
+monitor_printf(mon, "%*scommon features:\n", HMP_INFO_VIRTIO_INDENT, "");
+
+for (idx = 0; idx < ARRAY_SIZE(features); idx++) {
+const char *ack = virtio_vdev_has_feature(vdev, features[idx])
+? "acked" : "";
+
+if (!virtio_host_has_feature(vdev, features[idx])) {
+continue;
+}
+monitor_printf(mon, "%*s%*s%*s\n", HMP_INFO_VIRTIO_INDENT, "",
+ HMP_INFO_VIRTIO_FIELD, names[idx],
+ HMP_INFO_VIRTIO_FIELD, ack);
+}
+}
+
+static void hmp_info_virtio_print_specific_features(Monitor *mon,
+VirtIODevice *vdev)
+{
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+unsigned idx;
+
+if (!vdc->get_feature_name) {
+return;
+}
+
+monitor_printf(mon, "%*sspecific features:\n", HMP_INFO_VIRTIO_INDENT, &q
Re: [Qemu-block] [Qemu-devel] [PATCH v2] virtio: introduce `info virtio' hmp command
On 09/27/2017 06:53 PM, Dr. David Alan Gilbert wrote:
> * Jan Dakinevich (jan.dakinev...@virtuozzo.com) wrote:
>> The command is intended for exposing device specific virtio feature bits
>> and their negotiation status. It is convenient and useful for debug
>> purpose.
>>
>> Names of features are taken from a devices via get_feature_name() within
>> VirtioDeviceClass. If certain device doesn't implement it, the command
>> will print only hexadecimal value of feature mask.
>>
>> Cc: Denis V. Lunev
>> Signed-off-by: Jan Dakinevich
>> ---
>> v2:
>> * Moved hmp_info_virtio and stuff to hmp.c to avoid build error
>> * Added printing of device status
>> * Listed common virtio features
>>
>> v1: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html
>> ---
>> hmp-commands-info.hx| 14 +
>> hmp.c | 148
>>
>> hmp.h | 1 +
>> hw/block/virtio-blk.c | 20 ++
>> hw/char/virtio-serial-bus.c | 15 +
>> hw/display/virtio-gpu.c | 12
>> hw/net/virtio-net.c | 34 ++
>> hw/scsi/virtio-scsi.c | 15 +
>> hw/virtio/virtio-balloon.c | 15 +
>> hw/virtio/virtio-bus.c | 1 +
>> include/hw/virtio/virtio.h | 2 +
>> monitor.c | 1 +
>> 12 files changed, 278 insertions(+)
>>
>> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
>> index 4f1ece9..2550027 100644
>> --- a/hmp-commands-info.hx
>> +++ b/hmp-commands-info.hx
>> @@ -868,6 +868,20 @@ ETEXI
>> },
>>
>> STEXI
>> +@item info virtio
>> +@findex virtio
>> +Display guest and host fetures for all virtio devices.
>> +ETEXI
>> +
>> +{
>> +.name = "virtio",
>> +.args_type = "",
>> +.params = "",
>> +.help = "show virtio info",
>> +.cmd= hmp_info_virtio,
>> +},
>> +
>> +STEXI
>> @end table
>> ETEXI
>>
>> diff --git a/hmp.c b/hmp.c
>> index ace729d..009d808 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -43,6 +43,7 @@
>> #include "hw/intc/intc.h"
>> #include "migration/snapshot.h"
>> #include "migration/misc.h"
>> +#include "hw/virtio/virtio.h"
>>
>> #ifdef CONFIG_SPICE
>> #include
>> @@ -2894,3 +2895,150 @@ void hmp_info_memory_size_summary(Monitor *mon,
>> const QDict *qdict)
>> }
>> hmp_handle_error(mon, &err);
>> }
>> +
>> +#define HMP_INFO_VIRTIO_INDENT 2
>> +#define HMP_INFO_VIRTIO_FIELD 32
>> +
>> +static void hmp_info_virtio_print_status(Monitor *mon, VirtIODevice *vdev)
>> +{
>> +uint8_t status[] = {
>> +VIRTIO_CONFIG_S_ACKNOWLEDGE,
>> +VIRTIO_CONFIG_S_DRIVER,
>> +VIRTIO_CONFIG_S_DRIVER_OK,
>> +VIRTIO_CONFIG_S_FEATURES_OK,
>> +VIRTIO_CONFIG_S_NEEDS_RESET,
>> +VIRTIO_CONFIG_S_FAILED,
>> +};
>> +const char *names[] = {
>> +"acknowledge",
>> +"driver",
>> +"driver_ok",
>> +"features_ok",
>> +"needs_reset"
>> +"failed",
>> +};
>> +const char *comma = "";
>> +int idx;
>> +
>> +monitor_printf(mon, "%*sstatus: 0x%02"PRIx8" ",
>> + HMP_INFO_VIRTIO_INDENT, "", vdev->status);
>> +
>> +for (idx = 0; idx < ARRAY_SIZE(status); idx++) {
>> +if (!(vdev->status & status[idx])) {
>> +continue;
>> +}
>> +monitor_printf(mon, "%s%s", comma, names[idx]);
>> +if (names[idx]) {
>> +comma = ",";
>> +}
>> +}
>> +monitor_printf(mon, "\n");
>> +}
>> +
>> +static void hmp_info_virtio_print_common_features(Monitor *mon,
>> + VirtIODevice *vdev)
>> +{
>> +uint64_t features[] = {
>> +VIRTIO_RING_F_INDIRECT_DESC,
>> +VIRTIO_RING_F_EVENT_IDX,
>> +VIRTIO_F_NOTIFY_ON_EMPTY,
>> +VIRTIO_F_ANY_LAYOUT,
>> +VIRTIO_F_IOMMU_PLATFORM,
>> +VIRTIO_F_VERSION_1,
>> +};
>> +const char
[Qemu-block] [Qemu-devel] [PATCH v2] virtio: introduce `info virtio' hmp command
The command is intended for exposing device specific virtio feature bits
and their negotiation status. It is convenient and useful for debug
purpose.
Names of features are taken from a devices via get_feature_name() within
VirtioDeviceClass. If certain device doesn't implement it, the command
will print only hexadecimal value of feature mask.
Cc: Denis V. Lunev
Signed-off-by: Jan Dakinevich
---
v2:
* Moved hmp_info_virtio and stuff to hmp.c to avoid build error
* Added printing of device status
* Listed common virtio features
v1: http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg07247.html
---
hmp-commands-info.hx| 14 +
hmp.c | 148
hmp.h | 1 +
hw/block/virtio-blk.c | 20 ++
hw/char/virtio-serial-bus.c | 15 +
hw/display/virtio-gpu.c | 12
hw/net/virtio-net.c | 34 ++
hw/scsi/virtio-scsi.c | 15 +
hw/virtio/virtio-balloon.c | 15 +
hw/virtio/virtio-bus.c | 1 +
include/hw/virtio/virtio.h | 2 +
monitor.c | 1 +
12 files changed, 278 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 4f1ece9..2550027 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -868,6 +868,20 @@ ETEXI
},
STEXI
+@item info virtio
+@findex virtio
+Display guest and host fetures for all virtio devices.
+ETEXI
+
+{
+.name = "virtio",
+.args_type = "",
+.params = "",
+.help = "show virtio info",
+.cmd= hmp_info_virtio,
+},
+
+STEXI
@end table
ETEXI
diff --git a/hmp.c b/hmp.c
index ace729d..009d808 100644
--- a/hmp.c
+++ b/hmp.c
@@ -43,6 +43,7 @@
#include "hw/intc/intc.h"
#include "migration/snapshot.h"
#include "migration/misc.h"
+#include "hw/virtio/virtio.h"
#ifdef CONFIG_SPICE
#include
@@ -2894,3 +2895,150 @@ void hmp_info_memory_size_summary(Monitor *mon, const
QDict *qdict)
}
hmp_handle_error(mon, &err);
}
+
+#define HMP_INFO_VIRTIO_INDENT 2
+#define HMP_INFO_VIRTIO_FIELD 32
+
+static void hmp_info_virtio_print_status(Monitor *mon, VirtIODevice *vdev)
+{
+uint8_t status[] = {
+VIRTIO_CONFIG_S_ACKNOWLEDGE,
+VIRTIO_CONFIG_S_DRIVER,
+VIRTIO_CONFIG_S_DRIVER_OK,
+VIRTIO_CONFIG_S_FEATURES_OK,
+VIRTIO_CONFIG_S_NEEDS_RESET,
+VIRTIO_CONFIG_S_FAILED,
+};
+const char *names[] = {
+"acknowledge",
+"driver",
+"driver_ok",
+"features_ok",
+"needs_reset"
+"failed",
+};
+const char *comma = "";
+int idx;
+
+monitor_printf(mon, "%*sstatus: 0x%02"PRIx8" ",
+ HMP_INFO_VIRTIO_INDENT, "", vdev->status);
+
+for (idx = 0; idx < ARRAY_SIZE(status); idx++) {
+if (!(vdev->status & status[idx])) {
+continue;
+}
+monitor_printf(mon, "%s%s", comma, names[idx]);
+if (names[idx]) {
+comma = ",";
+}
+}
+monitor_printf(mon, "\n");
+}
+
+static void hmp_info_virtio_print_common_features(Monitor *mon,
+ VirtIODevice *vdev)
+{
+uint64_t features[] = {
+VIRTIO_RING_F_INDIRECT_DESC,
+VIRTIO_RING_F_EVENT_IDX,
+VIRTIO_F_NOTIFY_ON_EMPTY,
+VIRTIO_F_ANY_LAYOUT,
+VIRTIO_F_IOMMU_PLATFORM,
+VIRTIO_F_VERSION_1,
+};
+const char *names[] = {
+"indirect_desc",
+"event_idx",
+"notify_on_empty",
+"any_layout",
+"iommu_platform",
+"version_1",
+};
+int idx;
+
+monitor_printf(mon, "%*scommon features:\n", HMP_INFO_VIRTIO_INDENT, "");
+
+for (idx = 0; idx < ARRAY_SIZE(features); idx++) {
+const char *ack = vdev->guest_features & features[idx] ? "acked" : "";
+
+if (!(vdev->host_features & features[idx])) {
+continue;
+}
+monitor_printf(mon, "%*s%*s%*s\n", HMP_INFO_VIRTIO_INDENT, "",
+ HMP_INFO_VIRTIO_FIELD, names[idx],
+ HMP_INFO_VIRTIO_FIELD, ack);
+}
+}
+
+static void hmp_info_virtio_print_specific_features(Monitor *mon,
+VirtIODevice *vdev)
+{
+VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
+int idx;
+
+if (!vdc->get_feature_name) {
+return;
+}
+
+monitor_printf(mon, "%*sspecific features:\n", HMP_INFO_VIRTIO_INDENT, "");
+
+for (idx = 0; idx < 64; idx++) {
+co
[Qemu-block] [PATCH] virtio: introduce `info virtio' hmp command
The command is intended for exposing device specific virtio feature bits
and their negotiation status. It is convenient and useful for debug
purpose.
Names of features are taken from a devices via get_feature_name() within
VirtioDeviceClass. If certain device doesn't implement it, the command
will print only hexadecimal value of feature mask.
Cc: Denis V. Lunev
Signed-off-by: Jan Dakinevich
---
The patch suggests an another approach for this:
"virtio: show guest features in 'info qtree'"
http://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg01609.html
---
hmp-commands-info.hx | 14 ++
hw/block/virtio-blk.c | 20 ++
hw/char/virtio-serial-bus.c| 15 +++
hw/display/virtio-gpu.c| 12 +
hw/net/virtio-net.c| 34
hw/scsi/virtio-scsi.c | 15 +++
hw/virtio/virtio-balloon.c | 15 +++
hw/virtio/virtio-bus.c | 59 ++
include/hw/virtio/virtio-bus.h | 2 ++
include/hw/virtio/virtio.h | 2 ++
monitor.c | 1 +
11 files changed, 189 insertions(+)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 4f1ece9..2550027 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -868,6 +868,20 @@ ETEXI
},
STEXI
+@item info virtio
+@findex virtio
+Display guest and host fetures for all virtio devices.
+ETEXI
+
+{
+.name = "virtio",
+.args_type = "",
+.params = "",
+.help = "show virtio info",
+.cmd= hmp_info_virtio,
+},
+
+STEXI
@end table
ETEXI
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 05d1440..5856640 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1017,6 +1017,25 @@ static Property virtio_blk_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static const char *virtio_blk_get_feature_name(VirtIODevice *vdev, int feature)
+{
+static const char *names[] = {
+[VIRTIO_BLK_F_BARRIER] = "barrier",
+[VIRTIO_BLK_F_SIZE_MAX] = "size_max",
+[VIRTIO_BLK_F_SEG_MAX] = "seg_max",
+[VIRTIO_BLK_F_RO] = "ro",
+[VIRTIO_BLK_F_BLK_SIZE] = "blk_size",
+[VIRTIO_BLK_F_SCSI] = "scsi",
+[VIRTIO_BLK_F_TOPOLOGY] = "topology",
+[VIRTIO_BLK_F_FLUSH] = "flush",
+[VIRTIO_BLK_F_MQ] = "mq",
+};
+if (feature >= ARRAY_SIZE(names)) {
+return NULL;
+}
+return names[feature];
+}
+
static void virtio_blk_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1030,6 +1049,7 @@ static void virtio_blk_class_init(ObjectClass *klass,
void *data)
vdc->get_config = virtio_blk_update_config;
vdc->set_config = virtio_blk_set_config;
vdc->get_features = virtio_blk_get_features;
+vdc->get_feature_name = virtio_blk_get_feature_name;
vdc->set_status = virtio_blk_set_status;
vdc->reset = virtio_blk_reset;
vdc->save = virtio_blk_save_device;
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 9470bd7..a6ccb6a 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1156,6 +1156,20 @@ static Property virtio_serial_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static const char *virtio_serial_get_feature_name(VirtIODevice *vdev,
+ int feature)
+{
+static const char *names[] = {
+[VIRTIO_CONSOLE_F_SIZE] = "size",
+[VIRTIO_CONSOLE_F_MULTIPORT] = "multiport",
+[VIRTIO_CONSOLE_F_EMERG_WRITE] = "emerg_write",
+};
+if (feature >= ARRAY_SIZE(names)) {
+return NULL;
+}
+return names[feature];
+}
+
static void virtio_serial_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1170,6 +1184,7 @@ static void virtio_serial_class_init(ObjectClass *klass,
void *data)
vdc->realize = virtio_serial_device_realize;
vdc->unrealize = virtio_serial_device_unrealize;
vdc->get_features = get_features;
+vdc->get_feature_name = virtio_serial_get_feature_name;
vdc->get_config = get_config;
vdc->set_config = set_config;
vdc->set_status = set_status;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 3a8f1e1..860d2b3 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1307,6 +1307,17 @@ static Property virtio_gpu_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
+static const char *virtio_gpu_get_feature_name(VirtIODevice *vdev, int feature)
+{
+static const char *names[] = {
+[VIRTIO_GPU_F_VIRGL] = "virgl",
+};
+if (feature >= ARRAY_
