Re: [Qemu-devel] [PATCH 2/8] qmp: create qmp_savevm command
On 01/19/2016 09:11 PM, Markus Armbruster wrote:
"Denis V. Lunev" writes:
On 01/18/2016 06:58 PM, Markus Armbruster wrote:
"Denis V. Lunev" writes:
'name' attribute is made mandatory in distinction with HMP command.
The patch also moves hmp_savevm implementation into hmp.c. This function
is just a simple wrapper now and does not have knowledge about
migration internals.
[...]
diff --git a/qapi-schema.json b/qapi-schema.json
index 2e31733..09d1a1a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4054,3 +4054,16 @@
##
{ 'enum': 'ReplayMode',
'data': [ 'none', 'record', 'play' ] }
+
+##
+# @savevm
+#
+# Save a VM snapshot. Old snapshot with the same name will be deleted if
exists.
+#
+# @name: identifier of a snapshot to be created
+#
+# Returns: Nothing on success
+#
+# Since 2.6
+##
+{ 'command': 'savevm', 'data': {'name': 'str'} }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index db072a6..b7851e1 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -4795,3 +4795,28 @@ Example:
{"type": 0, "out-pport": 0, "pport": 0, "vlan-id": 3840,
"pop-vlan": 1, "id": 251658240}
]}
+
+EQMP
+
+SQMP
+savevm
+--
+
+Save a VM snapshot. Old snapshot with the same name will be deleted if exists.
+
+Arguments:
+
+- "name": snapshot name
+
+Example:
+
+-> { "execute": "savevm", "arguments": { "name": "snapshot1" } }
+<- { "return": {} }
+
+EQMP
+
+{
+.name = "savevm",
+.args_type = "name:s",
+.mhandler.cmd_new = qmp_marshal_savevm,
+},
A snapshot has a tag (QEMUSnapshotInfo member name) and an ID
(QEMUSnapshotInfo member id_str).
HMP's name arguments are overloaded: they're matched both against tag
and ID. Unwisely chosen tags can create ambiguity. Example:
(qemu) savevm 2
(qemu) savevm
(qemu) info snapshots
IDTAG VM SIZEDATE VM CLOCK
1 2 1.7M 2016-01-18 16:56:31 00:00:00.000
2 vm-20160118165641 1.7M 2016-01-18 16:56:41 00:00:00.000
Care to guess which one we get when we ask for "2"?
I think we want separate, unoverloaded arguments for QMP.
I think there is no need to. Name is now absolutely mandatory.
Thus for new snapshots we will have 'name' specified and we
will be bound to name only.
'id' will be used for old VMs and this is convenience
layer to make old 'id' only snaphosts accessible
through new interface in the same way as old.
The overloaded interface you propose is more complex than it seems. You
hide the complexity by not documenting its workings. Not even to the
(insufficient!) degree the HMP interface documents how its overloaded
name parameters work.
Merely copying over the HMP documentation won't cut it. The bar for new
QMP interfaces is a fair bit higher than "no worse than HMP". The new
interface must reasonably sane for *QMP*, and sufficiently documented.
If we can't make a sane QMP interface, I'd rather have no QMP interface.
However, I believe we *can* make a sane QMP interface if we put in the
design work.
The design work must start with a review of what we're trying to
accomplish, and how to fit it into the rest of the system. Here's my
attempt. Since my knowledge on snapshots is rather superficial, I'm
cc'ing Kevin for additional snapshot expertise. Kevin, please correct
me when I talk nonsense. I'm further cc'ing Eric and Peter for the
management layer perspective.
A point-in-time snapshot of a system consists of a snapshot of its
machine state and snapshots of its storage. All the snapshots need to
be made at the same point in time for the result to be consistent.
Snapshots of read-only storage carry no information and are commonly
omitted.
Isolated storage snapshots can make sense, but snapshotting the machine
state without also snapshotting the machine's storage doesn't sound
useful to me.
Both storage and machine state snapshots come in two flavours: internal
and external.
External ones can be made with any block backend, but internal storage
snapshots work only with certain formats, notably qcow2. QMP supports
both kinds of storage snapshots.
Both kinds of storage snapshots need exclusive access while they work.
They're relatively quick, but the delay could be noticable for large
internal snapshots, and perhaps for external snapshots on really slow
storage.
Internal machine state snapshots are currently only available via HMP's
savevm, which integrates internal machine state and storage snapshots.
This is non-live, i.e. the guest is stopped while the snapshot gets
saved. I figure we could make it live if we really wanted to. Another
instance of the emerging background job concept.
On the implementation level, QCOW2 can't currently store a machine state
snapshot without also storing a storage snapshot. I guess we could
change this if we really wanted to.
External machine state snapshots are basically migrate to file.
S
Re: [Qemu-devel] [PATCH v2 3/9] ipmi: replace *_MAXCMD defines
On Thu, 21 Jan 2016 18:18:48 +0100
Cédric Le Goater wrote:
> ARRAY_SIZE() is simple to use and removes the need to pre-define
> the size of the command arrays.
>
> Signed-off-by: Cédric Le Goater
> ---
Much nicer !
Reviewed-by: Greg Kurz
> hw/ipmi/ipmi_bmc_sim.c | 21 -
> 1 file changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index e42c7e86c344..fc596a548df7 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -30,14 +30,12 @@
> #include "qemu/error-report.h"
>
> #define IPMI_NETFN_CHASSIS0x00
> -#define IPMI_NETFN_CHASSIS_MAXCMD 0x03
>
> #define IPMI_CMD_GET_CHASSIS_CAPABILITIES 0x00
> #define IPMI_CMD_GET_CHASSIS_STATUS 0x01
> #define IPMI_CMD_CHASSIS_CONTROL 0x02
>
> #define IPMI_NETFN_SENSOR_EVENT 0x04
> -#define IPMI_NETFN_SENSOR_EVENT_MAXCMD0x2e
>
> #define IPMI_CMD_SET_SENSOR_EVT_ENABLE0x28
> #define IPMI_CMD_GET_SENSOR_EVT_ENABLE0x29
> @@ -46,7 +44,6 @@
> #define IPMI_CMD_GET_SENSOR_READING 0x2d
>
> /* #define IPMI_NETFN_APP 0x06 In ipmi.h */
> -#define IPMI_NETFN_APP_MAXCMD 0x36
>
> #define IPMI_CMD_GET_DEVICE_ID0x01
> #define IPMI_CMD_COLD_RESET 0x02
> @@ -63,7 +60,6 @@
> #define IPMI_CMD_READ_EVT_MSG_BUF 0x35
>
> #define IPMI_NETFN_STORAGE0x0a
> -#define IPMI_NETFN_STORAGE_MAXCMD 0x4a
>
> #define IPMI_CMD_GET_SDR_REP_INFO 0x20
> #define IPMI_CMD_GET_SDR_REP_ALLOC_INFO 0x21
> @@ -1518,18 +1514,17 @@ static void get_sensor_reading(IPMIBmcSim *ibs,
> }
> }
>
> -static const IPMICmdHandler chassis_cmds[IPMI_NETFN_CHASSIS_MAXCMD] = {
> +static const IPMICmdHandler chassis_cmds[] = {
> [IPMI_CMD_GET_CHASSIS_CAPABILITIES] = chassis_capabilities,
> [IPMI_CMD_GET_CHASSIS_STATUS] = chassis_status,
> [IPMI_CMD_CHASSIS_CONTROL] = chassis_control
> };
> static const IPMINetfn chassis_netfn = {
> -.cmd_nums = IPMI_NETFN_CHASSIS_MAXCMD,
> +.cmd_nums = ARRAY_SIZE(chassis_cmds),
> .cmd_handlers = chassis_cmds
> };
>
> -static const IPMICmdHandler
> -sensor_event_cmds[IPMI_NETFN_SENSOR_EVENT_MAXCMD] = {
> +static const IPMICmdHandler sensor_event_cmds[] = {
> [IPMI_CMD_SET_SENSOR_EVT_ENABLE] = set_sensor_evt_enable,
> [IPMI_CMD_GET_SENSOR_EVT_ENABLE] = get_sensor_evt_enable,
> [IPMI_CMD_REARM_SENSOR_EVTS] = rearm_sensor_evts,
> @@ -1537,11 +1532,11 @@ sensor_event_cmds[IPMI_NETFN_SENSOR_EVENT_MAXCMD] = {
> [IPMI_CMD_GET_SENSOR_READING] = get_sensor_reading
> };
> static const IPMINetfn sensor_event_netfn = {
> -.cmd_nums = IPMI_NETFN_SENSOR_EVENT_MAXCMD,
> +.cmd_nums = ARRAY_SIZE(sensor_event_cmds),
> .cmd_handlers = sensor_event_cmds
> };
>
> -static const IPMICmdHandler app_cmds[IPMI_NETFN_APP_MAXCMD] = {
> +static const IPMICmdHandler app_cmds[] = {
> [IPMI_CMD_GET_DEVICE_ID] = get_device_id,
> [IPMI_CMD_COLD_RESET] = cold_reset,
> [IPMI_CMD_WARM_RESET] = warm_reset,
> @@ -1557,11 +1552,11 @@ static const IPMICmdHandler
> app_cmds[IPMI_NETFN_APP_MAXCMD] = {
> [IPMI_CMD_GET_WATCHDOG_TIMER] = get_watchdog_timer,
> };
> static const IPMINetfn app_netfn = {
> -.cmd_nums = IPMI_NETFN_APP_MAXCMD,
> +.cmd_nums = ARRAY_SIZE(app_cmds),
> .cmd_handlers = app_cmds
> };
>
> -static const IPMICmdHandler storage_cmds[IPMI_NETFN_STORAGE_MAXCMD] = {
> +static const IPMICmdHandler storage_cmds[] = {
> [IPMI_CMD_GET_SDR_REP_INFO] = get_sdr_rep_info,
> [IPMI_CMD_RESERVE_SDR_REP] = reserve_sdr_rep,
> [IPMI_CMD_GET_SDR] = get_sdr,
> @@ -1577,7 +1572,7 @@ static const IPMICmdHandler
> storage_cmds[IPMI_NETFN_STORAGE_MAXCMD] = {
> };
>
> static const IPMINetfn storage_netfn = {
> -.cmd_nums = IPMI_NETFN_STORAGE_MAXCMD,
> +.cmd_nums = ARRAY_SIZE(storage_cmds),
> .cmd_handlers = storage_cmds
> };
>
[Qemu-devel] [PATCH] net: walk through filters reversely if traffic is outgress
Previously, if the netdev has more than one filters, the ingress
or outgress traffic pass the filter in the same order. this patch
is to make the outgress pass the filter in a reverse order
Signed-off-by: Wen Congyang
Signed-off-by: Li Zhijian
---
include/net/net.h | 4 +++-
net/filter.c | 21 +++--
net/net.c | 23 ++-
3 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index 7af3e15..1d807cc 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -79,6 +79,8 @@ typedef struct NetClientInfo {
SetVnetBE *set_vnet_be;
} NetClientInfo;
+QTAILQ_HEAD(NetFilterHead, NetFilterState);
+
struct NetClientState {
NetClientInfo *info;
int link_down;
@@ -92,7 +94,7 @@ struct NetClientState {
NetClientDestructor *destructor;
unsigned int queue_index;
unsigned rxfilter_notify_enabled:1;
-QTAILQ_HEAD(, NetFilterState) filters;
+struct NetFilterHead filters;
};
typedef struct NICState {
diff --git a/net/filter.c b/net/filter.c
index 5d90f83..17a8398 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -34,6 +34,22 @@ ssize_t qemu_netfilter_receive(NetFilterState *nf,
return 0;
}
+static NetFilterState *netfilter_next(NetFilterState *nf,
+ NetFilterDirection dir)
+{
+NetFilterState *next;
+
+if (dir == NET_FILTER_DIRECTION_TX) {
+/* forward walk through filters */
+next = QTAILQ_NEXT(nf, next);
+} else {
+/* reverse order */
+next = QTAILQ_PREV(nf, NetFilterHead, next);
+}
+
+return next;
+}
+
ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
unsigned flags,
const struct iovec *iov,
@@ -43,7 +59,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
int ret = 0;
int direction;
NetFilterState *nf = opaque;
-NetFilterState *next = QTAILQ_NEXT(nf, next);
+NetFilterState *next = NULL;
if (!sender || !sender->peer) {
/* no receiver, or sender been deleted, no need to pass it further */
@@ -61,6 +77,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
direction = nf->direction;
}
+next = netfilter_next(nf, direction);
while (next) {
/*
* if qemu_netfilter_pass_to_next been called, means that
@@ -73,7 +90,7 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
if (ret) {
return ret;
}
-next = QTAILQ_NEXT(next, next);
+next = netfilter_next(next, direction);
}
/*
diff --git a/net/net.c b/net/net.c
index 87dd356..05ec996 100644
--- a/net/net.c
+++ b/net/net.c
@@ -580,11 +580,24 @@ static ssize_t filter_receive_iov(NetClientState *nc,
ssize_t ret = 0;
NetFilterState *nf = NULL;
-QTAILQ_FOREACH(nf, &nc->filters, next) {
-ret = qemu_netfilter_receive(nf, direction, sender, flags, iov,
- iovcnt, sent_cb);
-if (ret) {
-return ret;
+assert(direction == NET_FILTER_DIRECTION_TX ||
+ direction == NET_FILTER_DIRECTION_RX);
+
+if (direction == NET_FILTER_DIRECTION_TX) {
+QTAILQ_FOREACH(nf, &nc->filters, next) {
+ret = qemu_netfilter_receive(nf, direction, sender, flags, iov,
+ iovcnt, sent_cb);
+if (ret) {
+return ret;
+}
+}
+} else {
+QTAILQ_FOREACH_REVERSE(nf, &nc->filters, NetFilterHead, next) {
+ret = qemu_netfilter_receive(nf, direction, sender, flags, iov,
+ iovcnt, sent_cb);
+if (ret) {
+return ret;
+}
}
}
--
2.5.0
Re: [Qemu-devel] [PATCH 2/2] net: netmap: avoid mmap() when ports use the same shared memory area
No, you're right, nm_mmap() is not defined for all netmap versions.
It will take a while for us to decide how to manage this. Would it be
possible to go ahead with patch 1/2 only (I will reissue the patch
series)?
Cheers,
Vincenzo
2016-01-14 4:25 GMT+01:00 Jason Wang :
>
>
> On 01/08/2016 09:15 PM, Vincenzo Maffione wrote:
>> With this patch, nm_open() does not mmap() the netmap device. This
>> operation is performed separately only if the memory area of the
>> port just opened was not known before.
>> A global list of netmap clients is kept to check when matches
>> occur.
>>
>> Signed-off-by: Vincenzo Maffione
>> ---
>> net/netmap.c | 38 +-
>> 1 file changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/netmap.c b/net/netmap.c
>> index 27295ab..6a4c01c 100644
>> --- a/net/netmap.c
>> +++ b/net/netmap.c
>> @@ -49,8 +49,12 @@ typedef struct NetmapState {
>> boolwrite_poll;
>> struct ioveciov[IOV_MAX];
>> int vnet_hdr_len; /* Current virtio-net header length.
>> */
>> +QTAILQ_ENTRY(NetmapState) next;
>> } NetmapState;
>>
>> +static QTAILQ_HEAD(, NetmapState) netmap_clients =
>> + QTAILQ_HEAD_INITIALIZER(netmap_clients);
>> +
>> #ifndef __FreeBSD__
>> #define pkt_copy bcopy
>> #else
>> @@ -78,6 +82,23 @@ pkt_copy(const void *_src, void *_dst, int l)
>> #endif /* __FreeBSD__ */
>>
>> /*
>> + * find nm_desc parent with same allocator
>> + */
>> +static struct nm_desc *
>> +netmap_find_memory(struct nm_desc *nmd)
>> +{
>> +NetmapState *s;
>> +
>> +QTAILQ_FOREACH(s, &netmap_clients, next) {
>> +if (nmd->req.nr_arg2 == s->nmd->req.nr_arg2) {
>> +return s->nmd;
>> +}
>> +}
>> +
>> +return NULL;
>> +}
>> +
>> +/*
>> * Open a netmap device. We assume there is only one queue
>> * (which is the case for the VALE bridge).
>> */
>> @@ -86,10 +107,11 @@ static struct nm_desc *netmap_open(const
>> NetdevNetmapOptions *nm_opts,
>> {
>> struct nm_desc *nmd;
>> struct nmreq req;
>> +int ret;
>>
>> memset(&req, 0, sizeof(req));
>>
>> -nmd = nm_open(nm_opts->ifname, &req, NETMAP_NO_TX_POLL,
>> +nmd = nm_open(nm_opts->ifname, &req, NETMAP_NO_TX_POLL |
>> NM_OPEN_NO_MMAP,
>>NULL);
>> if (nmd == NULL) {
>> error_setg_errno(errp, errno, "Failed to nm_open() %s",
>> @@ -97,6 +119,17 @@ static struct nm_desc *netmap_open(const
>> NetdevNetmapOptions *nm_opts,
>> return NULL;
>> }
>>
>> +/* Check if we already have a nm_desc that uses the same memory as the
>> one
>> + * just opened, so that nm_mmap() can skip mmap() and inherit from
>> parent.
>> + */
>> +ret = nm_mmap(nmd, netmap_find_memory(nmd));
>
> Looks like I could not find nm_mmap() definition in neither qemu or
> freebsd source. Is there anything missed?
>
>> +if (ret) {
>> +error_setg_errno(errp, errno, "Failed to nm_mmap() %s",
>> + nm_opts->ifname);
>> +nm_close(nmd);
>> +return NULL;
>> +}
>> +
>> return nmd;
>> }
>>
>> @@ -321,6 +354,8 @@ static void netmap_cleanup(NetClientState *nc)
>> netmap_poll(nc, false);
>> nm_close(s->nmd);
>> s->nmd = NULL;
>> +
>> +QTAILQ_REMOVE(&netmap_clients, s, next);
>> }
>>
>> /* Offloading manipulation support callbacks. */
>> @@ -423,6 +458,7 @@ int net_init_netmap(const NetClientOptions *opts,
>> s->rx = NETMAP_RXRING(nmd->nifp, 0);
>> s->vnet_hdr_len = 0;
>> pstrcpy(s->ifname, sizeof(s->ifname), netmap_opts->ifname);
>> +QTAILQ_INSERT_TAIL(&netmap_clients, s, next);
>> netmap_read_poll(s, true); /* Initially only poll for reads. */
>>
>> return 0;
>
--
Vincenzo Maffione
[Qemu-devel] [PATCH RFC 7/7] net/filter: prevent the default filter to be deleted
Signed-off-by: zhanghailiang
---
net/filter.c | 8
1 file changed, 8 insertions(+)
diff --git a/net/filter.c b/net/filter.c
index a126a3b..4aafff0 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -323,11 +323,19 @@ static void netfilter_finalize(Object *obj)
g_free(nf->netdev_id);
}
+static bool netfilter_can_be_deleted(UserCreatable *uc, Error **errp)
+{
+NetFilterState *nf = NETFILTER(uc);
+/* Forbid the default filter to be deleted */
+return !nf->is_default;
+}
+
static void netfilter_class_init(ObjectClass *oc, void *data)
{
UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
ucc->complete = netfilter_complete;
+ucc->can_be_deleted = netfilter_can_be_deleted;
}
static const TypeInfo netfilter_info = {
--
1.8.3.1
[Qemu-devel] [PATCH RFC 0/7] Netfilter: Add each netdev a default filter
This series is a prerequisite for COLO, here we add each netdev a default buffer filter, it is disabled by default, and has no side effect for delivering packets in net layer. Besides, patch 1 fixes the ouput information of 'info network' command for filter. zhanghailiang (7): net/filter: Fix the output information for command 'info network' net/filter: Add a 'status' property for filter object net/filter: Skip the disabled filter when delivering packets net/filter: Introduce a helper to add a filter to the netdev filter-buffer: Accept zero interval net/filter: Add a default filter to each netdev net/filter: prevent the default filter to be deleted include/net/filter.h | 25 +++- net/dump.c | 2 - net/filter-buffer.c | 10 net/filter.c | 163 +-- net/net.c| 27 - 5 files changed, 194 insertions(+), 33 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH RFC 2/7] net/filter: Add a 'status' property for filter object
With this property, users can control if this filter is 'enable'
or 'disable'. The default behavior for filter is enabled.
Signed-off-by: zhanghailiang
---
include/net/filter.h | 1 +
net/filter.c | 36
2 files changed, 37 insertions(+)
diff --git a/include/net/filter.h b/include/net/filter.h
index 8a20138..9ed5ec6 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -55,6 +55,7 @@ struct NetFilterState {
char *netdev_id;
NetClientState *netdev;
NetFilterDirection direction;
+bool enabled;
QTAILQ_ENTRY(NetFilterState) next;
};
diff --git a/net/filter.c b/net/filter.c
index 40254bd..f4933cc 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -117,8 +117,41 @@ static void netfilter_set_direction(Object *obj, int
direction, Error **errp)
nf->direction = direction;
}
+static char *netfilter_get_status(Object *obj, Error **errp)
+{
+NetFilterState *nf = NETFILTER(obj);
+
+if (nf->enabled) {
+return g_strdup("enable");
+} else {
+return g_strdup("disable");
+}
+}
+
+static void netfilter_set_status(Object *obj, const char *str, Error **errp)
+{
+NetFilterState *nf = NETFILTER(obj);
+
+if (!strcmp(str, "enable")) {
+nf->enabled = true;
+} else if (!strcmp(str, "disable")) {
+nf->enabled = false;
+} else {
+error_setg(errp, "Invalid value for netfilter status, "
+ "should be 'enable' or 'disable'");
+}
+}
+
static void netfilter_init(Object *obj)
{
+NetFilterState *nf = NETFILTER(obj);
+
+/*
+* If not configured with 'status' property, the default status
+* for netfilter will be enabled.
+*/
+nf->enabled = true;
+
object_property_add_str(obj, "netdev",
netfilter_get_netdev_id, netfilter_set_netdev_id,
NULL);
@@ -126,6 +159,9 @@ static void netfilter_init(Object *obj)
NetFilterDirection_lookup,
netfilter_get_direction, netfilter_set_direction,
NULL);
+object_property_add_str(obj, "status",
+netfilter_get_status, netfilter_set_status,
+NULL);
}
void netfilter_print_info(NetFilterState *nf, char *output_str, int size)
--
1.8.3.1
[Qemu-devel] [PATCH RFC 1/7] net/filter: Fix the output information for command 'info network'
The properties of netfilter object could be changed by 'qom-set'
command, but the output of 'info network' command is not updated,
because it got the old information through nf->info_str, it will
not be updated while we change the value of netfilter's property.
Here we split a the helper function that could colletct the output
information for filter, and also remove the useless member
'info_str' from struct NetFilterState.
Signed-off-by: zhanghailiang
---
include/net/filter.h | 3 ++-
net/filter.c | 47 ++-
net/net.c| 5 -
3 files changed, 32 insertions(+), 23 deletions(-)
diff --git a/include/net/filter.h b/include/net/filter.h
index 2deda36..8a20138 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -55,7 +55,6 @@ struct NetFilterState {
char *netdev_id;
NetClientState *netdev;
NetFilterDirection direction;
-char info_str[256];
QTAILQ_ENTRY(NetFilterState) next;
};
@@ -74,4 +73,6 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
int iovcnt,
void *opaque);
+void netfilter_print_info(NetFilterState *nf, char *output_str, int size);
+
#endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter.c b/net/filter.c
index 5d90f83..40254bd 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -128,6 +128,31 @@ static void netfilter_init(Object *obj)
NULL);
}
+void netfilter_print_info(NetFilterState *nf, char *output_str, int size)
+{
+char *str, *info;
+ObjectProperty *prop;
+ObjectPropertyIterator iter;
+StringOutputVisitor *ov;
+
+/* generate info str */
+object_property_iter_init(&iter, OBJECT(nf));
+while ((prop = object_property_iter_next(&iter))) {
+if (!strcmp(prop->name, "type")) {
+continue;
+}
+ov = string_output_visitor_new(false);
+object_property_get(OBJECT(nf), string_output_get_visitor(ov),
+prop->name, NULL);
+str = string_output_get_string(ov);
+string_output_visitor_cleanup(ov);
+info = g_strdup_printf(",%s=%s", prop->name, str);
+g_strlcat(output_str, info, size);
+g_free(str);
+g_free(info);
+}
+}
+
static void netfilter_complete(UserCreatable *uc, Error **errp)
{
NetFilterState *nf = NETFILTER(uc);
@@ -135,10 +160,7 @@ static void netfilter_complete(UserCreatable *uc, Error
**errp)
NetFilterClass *nfc = NETFILTER_GET_CLASS(uc);
int queues;
Error *local_err = NULL;
-char *str, *info;
-ObjectProperty *prop;
-ObjectPropertyIterator iter;
-StringOutputVisitor *ov;
+
if (!nf->netdev_id) {
error_setg(errp, "Parameter 'netdev' is required");
@@ -172,23 +194,6 @@ static void netfilter_complete(UserCreatable *uc, Error
**errp)
}
}
QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
-
-/* generate info str */
-object_property_iter_init(&iter, OBJECT(nf));
-while ((prop = object_property_iter_next(&iter))) {
-if (!strcmp(prop->name, "type")) {
-continue;
-}
-ov = string_output_visitor_new(false);
-object_property_get(OBJECT(nf), string_output_get_visitor(ov),
-prop->name, errp);
-str = string_output_get_string(ov);
-string_output_visitor_cleanup(ov);
-info = g_strdup_printf(",%s=%s", prop->name, str);
-g_strlcat(nf->info_str, info, sizeof(nf->info_str));
-g_free(str);
-g_free(info);
-}
}
static void netfilter_finalize(Object *obj)
diff --git a/net/net.c b/net/net.c
index 87dd356..87de7c0 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1198,9 +1198,12 @@ void print_net_client(Monitor *mon, NetClientState *nc)
}
QTAILQ_FOREACH(nf, &nc->filters, next) {
char *path = object_get_canonical_path_component(OBJECT(nf));
+char info[256] = { 0 };
+
+netfilter_print_info(nf, info, sizeof(info));
monitor_printf(mon, " - %s: type=%s%s\n", path,
object_get_typename(OBJECT(nf)),
- nf->info_str);
+ info);
g_free(path);
}
}
--
1.8.3.1
[Qemu-devel] [PATCH RFC 3/7] net/filter: Skip the disabled filter when delivering packets
If the filter is disabled, don't go through it.
Signed-off-by: zhanghailiang
---
include/net/filter.h | 5 +
net/net.c| 4
2 files changed, 9 insertions(+)
diff --git a/include/net/filter.h b/include/net/filter.h
index 9ed5ec6..d797ee4 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -74,6 +74,11 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
int iovcnt,
void *opaque);
+static inline bool qemu_need_skip_netfilter(NetFilterState *nf)
+{
+return nf->enabled ? false : true;
+}
+
void netfilter_print_info(NetFilterState *nf, char *output_str, int size);
#endif /* QEMU_NET_FILTER_H */
diff --git a/net/net.c b/net/net.c
index 87de7c0..ec43105 100644
--- a/net/net.c
+++ b/net/net.c
@@ -581,6 +581,10 @@ static ssize_t filter_receive_iov(NetClientState *nc,
NetFilterState *nf = NULL;
QTAILQ_FOREACH(nf, &nc->filters, next) {
+/* Don't go through filter if it is off */
+if (qemu_need_skip_netfilter(nf)) {
+continue;
+}
ret = qemu_netfilter_receive(nf, direction, sender, flags, iov,
iovcnt, sent_cb);
if (ret) {
--
1.8.3.1
[Qemu-devel] [PATCH RFC 4/7] net/filter: Introduce a helper to add a filter to the netdev
Signed-off-by: zhanghailiang
---
include/net/filter.h | 5 +
net/filter.c | 63
2 files changed, 68 insertions(+)
diff --git a/include/net/filter.h b/include/net/filter.h
index d797ee4..c7bd8f9 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -81,4 +81,9 @@ static inline bool qemu_need_skip_netfilter(NetFilterState
*nf)
void netfilter_print_info(NetFilterState *nf, char *output_str, int size);
+void netdev_add_filter(const char *netdev_id,
+ const char *filter_type,
+ const char *id,
+ Error **errp);
+
#endif /* QEMU_NET_FILTER_H */
diff --git a/net/filter.c b/net/filter.c
index f4933cc..4d96301 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -16,6 +16,10 @@
#include "qom/object_interfaces.h"
#include "qemu/iov.h"
#include "qapi/string-output-visitor.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp-output-visitor.h"
+#include "qapi/qmp-input-visitor.h"
+#include "monitor/monitor.h"
ssize_t qemu_netfilter_receive(NetFilterState *nf,
NetFilterDirection direction,
@@ -232,6 +236,65 @@ static void netfilter_complete(UserCreatable *uc, Error
**errp)
QTAILQ_INSERT_TAIL(&nf->netdev->filters, nf, next);
}
+/*
+* This will be used by COLO or MC FT, for which they will need
+* to buffer the packets of VM's net devices, Here we add a default
+* buffer filter for each netdev. The name of default buffer filter is
+* 'nop'
+*/
+void netdev_add_filter(const char *netdev_id,
+ const char *filter_type,
+ const char *id,
+ Error **errp)
+{
+QmpOutputVisitor *qov;
+QmpInputVisitor *qiv;
+Visitor *ov, *iv;
+QObject *obj = NULL;
+QDict *qdict;
+void *dummy = NULL;
+NetClientState *nc = qemu_find_netdev(netdev_id);
+Error *err = NULL;
+
+/* FIXME: Not support multiple queues */
+if (!nc || nc->queue_index > 1) {
+return;
+}
+/* Not support vhost-net */
+if (get_vhost_net(nc)) {
+return;
+}
+
+qov = qmp_output_visitor_new();
+ov = qmp_output_get_visitor(qov);
+visit_start_struct(ov, &dummy, NULL, NULL, 0, &err);
+if (err) {
+goto out;
+}
+visit_type_str(ov, &nc->name, "netdev", &err);
+if (err) {
+goto out;
+}
+visit_end_struct(ov, &err);
+if (err) {
+goto out;
+}
+obj = qmp_output_get_qobject(qov);
+g_assert(obj != NULL);
+qdict = qobject_to_qdict(obj);
+qmp_output_visitor_cleanup(qov);
+
+qiv = qmp_input_visitor_new(obj);
+iv = qmp_input_get_visitor(qiv);
+object_add(filter_type, id, qdict, iv, &err);
+qmp_input_visitor_cleanup(qiv);
+qobject_decref(obj);
+out:
+if (err) {
+error_propagate(errp, err);
+}
+}
+
static void netfilter_finalize(Object *obj)
{
NetFilterState *nf = NETFILTER(obj);
--
1.8.3.1
[Qemu-devel] [PATCH RFC 5/7] filter-buffer: Accept zero interval
We may want to accept zero interval when VM FT solutions like MC
or COLO use this filter to release packets on demand.
Signed-off-by: zhanghailiang
Reviewed-by: Yang Hongyang
---
net/filter-buffer.c | 10 --
1 file changed, 10 deletions(-)
diff --git a/net/filter-buffer.c b/net/filter-buffer.c
index 57be149..12e0c87 100644
--- a/net/filter-buffer.c
+++ b/net/filter-buffer.c
@@ -103,16 +103,6 @@ static void filter_buffer_setup(NetFilterState *nf, Error
**errp)
{
FilterBufferState *s = FILTER_BUFFER(nf);
-/*
- * We may want to accept zero interval when VM FT solutions like MC
- * or COLO use this filter to release packets on demand.
- */
-if (!s->interval) {
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "interval",
- "a non-zero interval");
-return;
-}
-
s->incoming_queue = qemu_new_net_queue(qemu_netfilter_pass_to_next, nf);
if (s->interval) {
timer_init_us(&s->release_timer, QEMU_CLOCK_VIRTUAL,
--
1.8.3.1
[Qemu-devel] [PATCH RFC 6/7] net/filter: Add a default filter to each netdev
We add each netdev a default buffer filter, which the name is
'nop', and the default buffer filter is disabled, so it has
no side effect for packets delivering in qemu net layer.
The default buffer filter can be used by COLO or Micro-checkpoint,
The reason we add the default filter is we hope to support
hot add network during COLO state in future.
Signed-off-by: zhanghailiang
---
include/net/filter.h | 11 +++
net/dump.c | 2 --
net/filter.c | 15 ++-
net/net.c| 18 ++
4 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/include/net/filter.h b/include/net/filter.h
index c7bd8f9..2043609 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -22,6 +22,16 @@
#define NETFILTER_CLASS(klass) \
OBJECT_CLASS_CHECK(NetFilterClass, (klass), TYPE_NETFILTER)
+#define DEFAULT_FILTER_NAME "nop"
+
+#define TYPE_FILTER_BUFFER "filter-buffer"
+#define TYPE_FILTER_DUMP "filter-dump"
+
+#define NETFILTER_ID_BUFFER 1
+#define NETFILTER_ID_DUMP 2
+
+extern const char *const netfilter_type_lookup[];
+
typedef void (FilterSetup) (NetFilterState *nf, Error **errp);
typedef void (FilterCleanup) (NetFilterState *nf);
/*
@@ -55,6 +65,7 @@ struct NetFilterState {
char *netdev_id;
NetClientState *netdev;
NetFilterDirection direction;
+bool is_default;
bool enabled;
QTAILQ_ENTRY(NetFilterState) next;
};
diff --git a/net/dump.c b/net/dump.c
index 88d9582..82727a6 100644
--- a/net/dump.c
+++ b/net/dump.c
@@ -229,8 +229,6 @@ int net_init_dump(const NetClientOptions *opts, const char
*name,
/* Dumping via filter */
-#define TYPE_FILTER_DUMP "filter-dump"
-
#define FILTER_DUMP(obj) \
OBJECT_CHECK(NetFilterDumpState, (obj), TYPE_FILTER_DUMP)
diff --git a/net/filter.c b/net/filter.c
index 4d96301..a126a3b 100644
--- a/net/filter.c
+++ b/net/filter.c
@@ -21,6 +21,11 @@
#include "qapi/qmp-input-visitor.h"
#include "monitor/monitor.h"
+const char *const netfilter_type_lookup[] = {
+[NETFILTER_ID_BUFFER] = TYPE_FILTER_BUFFER,
+[NETFILTER_ID_DUMP] = TYPE_FILTER_DUMP,
+};
+
ssize_t qemu_netfilter_receive(NetFilterState *nf,
NetFilterDirection direction,
NetClientState *sender,
@@ -200,7 +205,7 @@ static void netfilter_complete(UserCreatable *uc, Error
**errp)
NetFilterClass *nfc = NETFILTER_GET_CLASS(uc);
int queues;
Error *local_err = NULL;
-
+char *path = object_get_canonical_path_component(OBJECT(nf));
if (!nf->netdev_id) {
error_setg(errp, "Parameter 'netdev' is required");
@@ -225,6 +230,14 @@ static void netfilter_complete(UserCreatable *uc, Error
**errp)
}
nf->netdev = ncs[0];
+nf->is_default = !strcmp(path, DEFAULT_FILTER_NAME);
+/*
+* For the default buffer filter, it will be disabled by default,
+* So it will not buffer any packets.
+*/
+if (nf->is_default) {
+nf->enabled = false;
+}
if (nfc->setup) {
nfc->setup(nf, &local_err);
diff --git a/net/net.c b/net/net.c
index ec43105..9630234 100644
--- a/net/net.c
+++ b/net/net.c
@@ -76,6 +76,12 @@ const char *host_net_devices[] = {
int default_net = 1;
+/*
+ * FIXME: Export this with an option for users to control
+ * this with comand line ?
+ */
+int default_netfilter = NETFILTER_ID_BUFFER;
+
/***/
/* network device redirectors */
@@ -1032,6 +1038,18 @@ static int net_client_init1(const void *object, int
is_netdev, Error **errp)
}
return -1;
}
+
+if (is_netdev) {
+const Netdev *netdev = object;
+/*
+* Here we add each netdev a default filter whose name is 'nop',
+* it will disabled by default, Users can enable it when necessary.
+*/
+netdev_add_filter(netdev->id,
+ netfilter_type_lookup[default_netfilter],
+ DEFAULT_FILTER_NAME,
+ errp);
+}
return 0;
}
--
1.8.3.1
Re: [Qemu-devel] [PATCH 0/6] Some improvements and small fixes for migration
ping ? On 2016/1/15 11:37, zhanghailiang wrote: Patch 1 ~ patch 4 are picked from COLO and live memory snapshot series, They are just small improvements for migration codes and have been reviewed by Dave. Patch 5, 6 are small fixes for migration releated documention. Please review. zhanghailiang (6): ram: Split host_from_stream_offset() into two helper functions migration: Rename the'file' member of MigrationState savevm: Split load vm state function qemu_loadvm_state migration/ram: Fix some helper functions' parameter to use PageSearchStatus qmp-commands.hx: Fix the missing options for migration parameters commands qmp-commands.hx: Document the missing options for migration capability commands include/exec/ram_addr.h | 8 ++- include/migration/migration.h | 2 +- migration/exec.c | 4 +- migration/fd.c| 4 +- migration/migration.c | 72 +-- migration/postcopy-ram.c | 6 +- migration/ram.c | 73 +++ migration/rdma.c | 2 +- migration/savevm.c| 158 +- migration/tcp.c | 4 +- migration/unix.c | 4 +- qmp-commands.hx | 33 +++-- 12 files changed, 222 insertions(+), 148 deletions(-)
Re: [Qemu-devel] [PATCH RFC 0/7] Netfilter: Add each netdev a default filter
Cc: Dr. David Alan Gilbert On 2016/1/22 16:36, zhanghailiang wrote: This series is a prerequisite for COLO, here we add each netdev a default buffer filter, it is disabled by default, and has no side effect for delivering packets in net layer. Besides, patch 1 fixes the ouput information of 'info network' command for filter. zhanghailiang (7): net/filter: Fix the output information for command 'info network' net/filter: Add a 'status' property for filter object net/filter: Skip the disabled filter when delivering packets net/filter: Introduce a helper to add a filter to the netdev filter-buffer: Accept zero interval net/filter: Add a default filter to each netdev net/filter: prevent the default filter to be deleted include/net/filter.h | 25 +++- net/dump.c | 2 - net/filter-buffer.c | 10 net/filter.c | 163 +-- net/net.c| 27 - 5 files changed, 194 insertions(+), 33 deletions(-)
Re: [Qemu-devel] [PATCH] e1000: eliminate infinite loops on out-of-bounds transfer start
On 01/22/16 07:15, Jason Wang wrote: > > > On 01/22/2016 02:11 PM, Michael Tokarev wrote: >> 22.01.2016 06:09, Jason Wang wrote: >>> On 01/19/2016 09:17 PM, Laszlo Ersek wrote: The start_xmit() and e1000_receive_iov() functions implement DMA transfers iterating over a set of descriptors that the guest's e1000 driver prepares: >> ... >>> Applied in my -net. >> This is CVE-2016-1981, btw. >> >> /mjt >> > > Add this into commit log. Thanks guys! Laszlo
Re: [Qemu-devel] [PATCH v2 03/13] block: Move block dirty bitmap code to separate files
On 20.01.2016 09:11, Fam Zheng wrote:
The only change is making bdrv_dirty_bitmap_truncate public. It is used in
block.c.
Signed-off-by: Fam Zheng
Reviewed-by: John Snow
---
block.c | 339 ---
block/Makefile.objs | 2 +-
block/dirty-bitmap.c | 366 +++
include/block/block.h| 35 +
include/block/dirty-bitmap.h | 43 +
5 files changed, 411 insertions(+), 374 deletions(-)
create mode 100644 block/dirty-bitmap.c
create mode 100644 include/block/dirty-bitmap.h
diff --git a/block.c b/block.c
index 54c37f9..ab79bfe 100644
--- a/block.c
+++ b/block.c
@@ -55,23 +55,6 @@
#include
#endif
-/**
- * A BdrvDirtyBitmap can be in three possible states:
- * (1) successor is NULL and disabled is false: full r/w mode
- * (2) successor is NULL and disabled is true: read only mode ("disabled")
- * (3) successor is set: frozen mode.
- * A frozen bitmap cannot be renamed, deleted, anonymized, cleared, set,
- * or enabled. A frozen bitmap can only abdicate() or reclaim().
- */
-struct BdrvDirtyBitmap {
-HBitmap *bitmap;/* Dirty sector bitmap implementation */
-BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
-char *name; /* Optional non-empty unique ID */
-int64_t size; /* Size of the bitmap (Number of sectors) */
-bool disabled; /* Bitmap is read-only */
-QLIST_ENTRY(BdrvDirtyBitmap) list;
-};
-
#define NOT_DONE 0x7fff /* used while emulated sync operation in progress
*/
struct BdrvStates bdrv_states = QTAILQ_HEAD_INITIALIZER(bdrv_states);
@@ -87,7 +70,6 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const
char *filename,
BlockDriverState *parent,
const BdrvChildRole *child_role, Error **errp);
-static void bdrv_dirty_bitmap_truncate(BlockDriverState *bs);
/* If non-zero, use only whitelisted block drivers */
static int use_bdrv_whitelist;
@@ -3373,327 +3355,6 @@ void bdrv_lock_medium(BlockDriverState *bs, bool locked)
}
}
-BdrvDirtyBitmap *bdrv_find_dirty_bitmap(BlockDriverState *bs, const char *name)
-{
-BdrvDirtyBitmap *bm;
-
-assert(name);
-QLIST_FOREACH(bm, &bs->dirty_bitmaps, list) {
-if (bm->name && !strcmp(name, bm->name)) {
-return bm;
-}
-}
-return NULL;
-}
-
-void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap)
-{
-assert(!bdrv_dirty_bitmap_frozen(bitmap));
-g_free(bitmap->name);
-bitmap->name = NULL;
-}
-
-BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
- uint32_t granularity,
- const char *name,
- Error **errp)
-{
-int64_t bitmap_size;
-BdrvDirtyBitmap *bitmap;
-uint32_t sector_granularity;
-
-assert((granularity & (granularity - 1)) == 0);
-
-if (name && bdrv_find_dirty_bitmap(bs, name)) {
-error_setg(errp, "Bitmap already exists: %s", name);
-return NULL;
-}
-sector_granularity = granularity >> BDRV_SECTOR_BITS;
-assert(sector_granularity);
-bitmap_size = bdrv_nb_sectors(bs);
-if (bitmap_size < 0) {
-error_setg_errno(errp, -bitmap_size, "could not get length of device");
-errno = -bitmap_size;
-return NULL;
-}
-bitmap = g_new0(BdrvDirtyBitmap, 1);
-bitmap->bitmap = hbitmap_alloc(bitmap_size, ctz32(sector_granularity));
-bitmap->size = bitmap_size;
-bitmap->name = g_strdup(name);
-bitmap->disabled = false;
-QLIST_INSERT_HEAD(&bs->dirty_bitmaps, bitmap, list);
-return bitmap;
-}
-
-bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
-{
-return bitmap->successor;
-}
-
-bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap)
-{
-return !(bitmap->disabled || bitmap->successor);
-}
-
-DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap)
-{
-if (bdrv_dirty_bitmap_frozen(bitmap)) {
-return DIRTY_BITMAP_STATUS_FROZEN;
-} else if (!bdrv_dirty_bitmap_enabled(bitmap)) {
-return DIRTY_BITMAP_STATUS_DISABLED;
-} else {
-return DIRTY_BITMAP_STATUS_ACTIVE;
-}
-}
-
-/**
- * Create a successor bitmap destined to replace this bitmap after an
operation.
- * Requires that the bitmap is not frozen and has no successor.
- */
-int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
- BdrvDirtyBitmap *bitmap, Error **errp)
-{
-uint64_t granularity;
-BdrvDirtyBitmap *child;
-
-if (bdrv_dirty_bitmap_frozen(bitmap)) {
-error_setg(errp, "Cannot create a successor for a bitmap that is "
- "currently frozen");
-return -1;
-}
-assert(!bitmap->successor);
-
-/* Create an anonymous successor */
-g
[Qemu-devel] [PATCH v2] qom, qmp, hmp, qapi: create qom-type-list for class properties
This patch adds support for qom-type-prop-list command to list object
class properties. A later patch will use this functionality to
implement x86_64-cpu properties.
Signed-off-by: Valentin Rakush
Cc: Luiz Capitulino
Cc: Eric Blake
Cc: Markus Armbruster
Cc: Andreas Färber
Cc: Daniel P. Berrange
---
V2: Fixes after first review
- changed command name from qom-type-list to qom-type-prop-list
- changed memory allocation from g_malloc0 to g_new0
- changed parameter name from path to typename
- fixed wordings and comments
- fixed source code formatting
- registered the command in monitor
hmp-commands.hx | 13 +
hmp.c| 26 ++
hmp.h| 1 +
include/qom/object.h | 31 +++
qapi-schema.json | 19 +++
qmp-commands.hx | 6 ++
qmp.c| 32
qom/object.c | 7 +++
8 files changed, 135 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index bb52e4d..0aca653 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1734,6 +1734,19 @@ Print QOM properties of object at location @var{path}
ETEXI
{
+.name = "qom-type-prop-list",
+.args_type = "typename:s?",
+.params = "typename",
+.help = "list QOM class properties",
+.mhandler.cmd = hmp_qom_type_prop_list,
+},
+
+STEXI
+@item qom-type-prop-list [@var{typename}]
+Print QOM properties of the type @var{typename}
+ETEXI
+
+{
.name = "qom-set",
.args_type = "path:s,property:s,value:s",
.params = "path property value",
diff --git a/hmp.c b/hmp.c
index 54f2620..4bad6a1 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2052,6 +2052,32 @@ void hmp_qom_list(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+void hmp_qom_type_prop_list(Monitor *mon, const QDict *qdict)
+{
+const char *path = qdict_get_try_str(qdict, "path");
+ObjectPropertyInfoList *list;
+Error *err = NULL;
+
+if (!path) {
+monitor_printf(mon, "/\n");
+return;
+}
+
+list = qmp_qom_type_prop_list(path, &err);
+if (!err) {
+ObjectPropertyInfoList *start = list;
+while (list) {
+ObjectPropertyInfo *value = list->value;
+
+monitor_printf(mon, "%s (%s)\n",
+ value->name, value->type);
+list = list->next;
+}
+qapi_free_ObjectPropertyInfoList(start);
+}
+hmp_handle_error(mon, &err);
+}
+
void hmp_qom_set(Monitor *mon, const QDict *qdict)
{
const char *path = qdict_get_str(qdict, "path");
diff --git a/hmp.h b/hmp.h
index a8c5b5a..8c12ebe 100644
--- a/hmp.h
+++ b/hmp.h
@@ -103,6 +103,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict);
void hmp_info_memdev(Monitor *mon, const QDict *qdict);
void hmp_info_memory_devices(Monitor *mon, const QDict *qdict);
void hmp_qom_list(Monitor *mon, const QDict *qdict);
+void hmp_qom_type_prop_list(Monitor *mon, const QDict *qdict);
void hmp_qom_set(Monitor *mon, const QDict *qdict);
void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/include/qom/object.h b/include/qom/object.h
index d0dafe9..0c8379d 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1013,6 +1013,37 @@ void object_property_iter_init(ObjectPropertyIterator
*iter,
*/
ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter);
+/**
+ * object_class_property_iter_init:
+ * @klass: the class owning the properties to be iterated over
+ *
+ * Initializes an iterator for traversing all properties
+ * registered against a class type and all parent classes.
+ *
+ * It is forbidden to modify the property list while iterating,
+ * whether removing or adding properties.
+ *
+ * NB For getting next property in the list the object related
+ * function object_property_iter_next is still used.
+ *
+ * Typical usage pattern would be
+ *
+ *
+ * Using object class property iterators
+ *
+ * ObjectProperty *prop;
+ * ObjectPropertyIterator iter;
+ *
+ * object_class property_iter_init(&iter, obj);
+ * while ((prop = object_property_iter_next(&iter))) {
+ * ... do something with prop ...
+ * }
+ *
+ *
+ */
+void object_class_property_iter_init(ObjectPropertyIterator *iter,
+ ObjectClass *klass);
+
void object_unparent(Object *obj);
/**
diff --git a/qapi-schema.json b/qapi-schema.json
index b3038b2..2e960db 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4081,3 +4081,22 @@
##
{ 'enum': 'ReplayMode',
'data': [ 'none', 'record', 'play' ] }
+
+##
+# @qom-type-prop-list:
+#
+# This command will list any properties of an object class
+# given its typename.
+#
+# @typename: the typename of the class. See @qom-list-types to check
+#
[Qemu-devel] [PATCH v3] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties
This patch adds support for qom-type-prop-list command to list object
class properties. A later patch will use this functionality to
implement x86_64-cpu properties.
Signed-off-by: Valentin Rakush
Cc: Luiz Capitulino
Cc: Eric Blake
Cc: Markus Armbruster
Cc: Andreas Färber
Cc: Daniel P. Berrange
---
V2: Fixes after first review
- changed command name from qom-type-list to qom-type-prop-list
- changed memory allocation from g_malloc0 to g_new0
- changed parameter name from path to typename
- fixed wordings and comments
- fixed source code formatting
- registered the command in monitor
V3: commit message fix
- commit message changed to reflect actual command name
hmp-commands.hx | 13 +
hmp.c| 26 ++
hmp.h| 1 +
include/qom/object.h | 31 +++
qapi-schema.json | 19 +++
qmp-commands.hx | 6 ++
qmp.c| 32
qom/object.c | 7 +++
8 files changed, 135 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index bb52e4d..0aca653 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1734,6 +1734,19 @@ Print QOM properties of object at location @var{path}
ETEXI
{
+.name = "qom-type-prop-list",
+.args_type = "typename:s?",
+.params = "typename",
+.help = "list QOM class properties",
+.mhandler.cmd = hmp_qom_type_prop_list,
+},
+
+STEXI
+@item qom-type-prop-list [@var{typename}]
+Print QOM properties of the type @var{typename}
+ETEXI
+
+{
.name = "qom-set",
.args_type = "path:s,property:s,value:s",
.params = "path property value",
diff --git a/hmp.c b/hmp.c
index 54f2620..4bad6a1 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2052,6 +2052,32 @@ void hmp_qom_list(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+void hmp_qom_type_prop_list(Monitor *mon, const QDict *qdict)
+{
+const char *path = qdict_get_try_str(qdict, "path");
+ObjectPropertyInfoList *list;
+Error *err = NULL;
+
+if (!path) {
+monitor_printf(mon, "/\n");
+return;
+}
+
+list = qmp_qom_type_prop_list(path, &err);
+if (!err) {
+ObjectPropertyInfoList *start = list;
+while (list) {
+ObjectPropertyInfo *value = list->value;
+
+monitor_printf(mon, "%s (%s)\n",
+ value->name, value->type);
+list = list->next;
+}
+qapi_free_ObjectPropertyInfoList(start);
+}
+hmp_handle_error(mon, &err);
+}
+
void hmp_qom_set(Monitor *mon, const QDict *qdict)
{
const char *path = qdict_get_str(qdict, "path");
diff --git a/hmp.h b/hmp.h
index a8c5b5a..8c12ebe 100644
--- a/hmp.h
+++ b/hmp.h
@@ -103,6 +103,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict);
void hmp_info_memdev(Monitor *mon, const QDict *qdict);
void hmp_info_memory_devices(Monitor *mon, const QDict *qdict);
void hmp_qom_list(Monitor *mon, const QDict *qdict);
+void hmp_qom_type_prop_list(Monitor *mon, const QDict *qdict);
void hmp_qom_set(Monitor *mon, const QDict *qdict);
void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/include/qom/object.h b/include/qom/object.h
index d0dafe9..0c8379d 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1013,6 +1013,37 @@ void object_property_iter_init(ObjectPropertyIterator
*iter,
*/
ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter);
+/**
+ * object_class_property_iter_init:
+ * @klass: the class owning the properties to be iterated over
+ *
+ * Initializes an iterator for traversing all properties
+ * registered against a class type and all parent classes.
+ *
+ * It is forbidden to modify the property list while iterating,
+ * whether removing or adding properties.
+ *
+ * NB For getting next property in the list the object related
+ * function object_property_iter_next is still used.
+ *
+ * Typical usage pattern would be
+ *
+ *
+ * Using object class property iterators
+ *
+ * ObjectProperty *prop;
+ * ObjectPropertyIterator iter;
+ *
+ * object_class property_iter_init(&iter, obj);
+ * while ((prop = object_property_iter_next(&iter))) {
+ * ... do something with prop ...
+ * }
+ *
+ *
+ */
+void object_class_property_iter_init(ObjectPropertyIterator *iter,
+ ObjectClass *klass);
+
void object_unparent(Object *obj);
/**
diff --git a/qapi-schema.json b/qapi-schema.json
index b3038b2..2e960db 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4081,3 +4081,22 @@
##
{ 'enum': 'ReplayMode',
'data': [ 'none', 'record', 'play' ] }
+
+##
+# @qom-type-prop-list:
+#
+# This command will list any properties of an object class
+# given its typename.
+#
+
Re: [Qemu-devel] [PATCH RFC 3/7] net/filter: Skip the disabled filter when delivering packets
On 01/22/2016 04:36 PM, zhanghailiang wrote:
> If the filter is disabled, don't go through it.
>
> Signed-off-by: zhanghailiang
> ---
> include/net/filter.h | 5 +
> net/net.c| 4
> 2 files changed, 9 insertions(+)
>
> diff --git a/include/net/filter.h b/include/net/filter.h
> index 9ed5ec6..d797ee4 100644
> --- a/include/net/filter.h
> +++ b/include/net/filter.h
> @@ -74,6 +74,11 @@ ssize_t qemu_netfilter_pass_to_next(NetClientState *sender,
> int iovcnt,
> void *opaque);
>
> +static inline bool qemu_need_skip_netfilter(NetFilterState *nf)
> +{
> +return nf->enabled ? false : true;
> +}
> +
> void netfilter_print_info(NetFilterState *nf, char *output_str, int size);
>
> #endif /* QEMU_NET_FILTER_H */
> diff --git a/net/net.c b/net/net.c
> index 87de7c0..ec43105 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -581,6 +581,10 @@ static ssize_t filter_receive_iov(NetClientState *nc,
> NetFilterState *nf = NULL;
>
> QTAILQ_FOREACH(nf, &nc->filters, next) {
> +/* Don't go through filter if it is off */
> +if (qemu_need_skip_netfilter(nf)) {
> +continue;
> +}
> ret = qemu_netfilter_receive(nf, direction, sender, flags, iov,
> iovcnt, sent_cb);
> if (ret) {
>
qemu_netfilter_pass_to_next() shoule also be updated.
Thanks
Wen Congyang
Re: [Qemu-devel] [PATCH v9 2/3] quorum: implement bdrv_add_child() and bdrv_del_child()
On Thu 21 Jan 2016 05:58:42 PM CET, Eric Blake wrote: In general, what do you do to make sure that the data in a new Quorum child is consistent with that of the rest of the array? >>> >>> Quorum can have more than one child when it starts. But we don't do >>> the similar check. So I don't think we should do such check here. >> >> Yes, but when you start a VM you can verify in advance that all >> members of the Quorum have the same data. If you do that on a running >> VM how can you know if the new disk is consistent with the others? > > User error if it is not. Just the same as it is user error if you > request a shallow drive-mirror but the destination is not the same > contents as the backing file. I don't think qemu has to protect us > from user error in this case. But the backing file is read-only so the user can guarantee that the destination has the same data before the shallow mirror. How do you do that in this case? Berto
Re: [Qemu-devel] [PATCH v2 4/4] block/qapi: Emit tray_open only if there is a tray
On Wed 20 Jan 2016 07:29:21 PM CET, Max Reitz wrote: > Signed-off-by: Max Reitz > --- > block/qapi.c | 2 +- > qapi/block-core.json | 4 ++-- > tests/qemu-iotests/067.out | 4 > 3 files changed, 3 insertions(+), 7 deletions(-) Reviewed-by: Alberto Garcia Berto
[Qemu-devel] [PATCH v3 0/3] target-arm: Add a few more S2 MMU input checks
From: "Edgar E. Iglesias" This adds the inputsize > pamax check and also fixes the startlevel checks to apply to the 64bit translations. Comments welcome! Cheers, Edgar ChangeLog: v2 -> v3: * Document pamax arg to check_s2_startlevel v1 -> v2: * inputsize > pmax check only applies to AArch64 * Fix commit message typo < should be > Edgar E. Iglesias (3): target-arm: Apply S2 MMU startlevel table size check to AArch64 target-arm: Make pamax an argument to check_s2_startlevel target-arm: Implement the S2 MMU inputsize > pamax check target-arm/helper.c | 39 --- 1 file changed, 24 insertions(+), 15 deletions(-) -- 1.9.1
[Qemu-devel] [PATCH v3 1/3] target-arm: Apply S2 MMU startlevel table size check to AArch64
From: "Edgar E. Iglesias"
The S2 starting level table size check applies to both AArch32
and AArch64. Move it to common code.
Reviewed-by: Alex Bennée
Signed-off-by: Edgar E. Iglesias
---
target-arm/helper.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index f956b67..8aedce9 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6581,11 +6581,19 @@ typedef enum {
static bool check_s2_startlevel(ARMCPU *cpu, bool is_aa64, int level,
int inputsize, int stride)
{
+const int grainsize = stride + 3;
+int startsizecheck;
+
/* Negative levels are never allowed. */
if (level < 0) {
return false;
}
+startsizecheck = inputsize - ((3 - level) * stride + grainsize);
+if (startsizecheck < 1 || startsizecheck > stride + 4) {
+return false;
+}
+
if (is_aa64) {
unsigned int pamax = arm_pamax(cpu);
@@ -6609,20 +6617,12 @@ static bool check_s2_startlevel(ARMCPU *cpu, bool
is_aa64, int level,
g_assert_not_reached();
}
} else {
-const int grainsize = stride + 3;
-int startsizecheck;
-
/* AArch32 only supports 4KB pages. Assert on that. */
assert(stride == 9);
if (level == 0) {
return false;
}
-
-startsizecheck = inputsize - ((3 - level) * stride + grainsize);
-if (startsizecheck < 1 || startsizecheck > stride + 4) {
-return false;
-}
}
return true;
}
--
1.9.1
[Qemu-devel] [PATCH v3 3/3] target-arm: Implement the S2 MMU inputsize > pamax check
From: "Edgar E. Iglesias"
Implement the inputsize > pamax check for Stage 2 translations.
We have multiple choices for how to respond to errors and
choose to fault.
Signed-off-by: Edgar E. Iglesias
---
target-arm/helper.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 2a6fa94..8901762 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6809,7 +6809,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
*/
int startlevel = extract32(tcr->raw_tcr, 6, 2);
unsigned int pamax = arm_pamax(cpu);
-bool ok;
+bool ok = true;
if (va_size == 32 || stride == 9) {
/* AArch32 or 4KB pages */
@@ -6819,9 +6819,17 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
level = 3 - startlevel;
}
-/* Check that the starting level is valid. */
-ok = check_s2_startlevel(cpu, va_size == 64, level,
- inputsize, stride, pamax);
+if (va_size == 64 &&
+inputsize > pamax &&
+(arm_el_is_aa64(env, 1) || inputsize > 40)) {
+/* We have multiple choices but choose to fault. */
+ok = false;
+}
+if (ok) {
+/* Check that the starting level is valid. */
+ok = check_s2_startlevel(cpu, va_size == 64, level,
+ inputsize, stride, pamax);
+}
if (!ok) {
/* AArch64 reports these as level 0 faults.
* AArch32 reports these as level 1 faults.
--
1.9.1
[Qemu-devel] [PATCH v3 2/3] target-arm: Make pamax an argument to check_s2_startlevel
From: "Edgar E. Iglesias"
Make pamax an argument to check_s2_startlevel in preparation
for future reuse.
No functional change.
Reviewed-by: Alex Bennée
Signed-off-by: Edgar E. Iglesias
---
target-arm/helper.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 8aedce9..2a6fa94 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -6575,11 +6575,13 @@ typedef enum {
* @startlevel: Suggested starting level
* @inputsize: Bitsize of IPAs
* @stride: Page-table stride (See the ARM ARM)
+ * @pamax: Implementation defined bit-width of physical addresses
*
* Returns true if the suggested starting level is OK and false otherwise.
*/
static bool check_s2_startlevel(ARMCPU *cpu, bool is_aa64, int level,
-int inputsize, int stride)
+int inputsize, int stride,
+unsigned int pamax)
{
const int grainsize = stride + 3;
int startsizecheck;
@@ -6595,8 +6597,6 @@ static bool check_s2_startlevel(ARMCPU *cpu, bool
is_aa64, int level,
}
if (is_aa64) {
-unsigned int pamax = arm_pamax(cpu);
-
switch (stride) {
case 13: /* 64KB Pages. */
if (level == 0 || (level == 1 && pamax <= 42)) {
@@ -6808,6 +6808,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
* VTCR_EL2.SL0 field (whose interpretation depends on the page size)
*/
int startlevel = extract32(tcr->raw_tcr, 6, 2);
+unsigned int pamax = arm_pamax(cpu);
bool ok;
if (va_size == 32 || stride == 9) {
@@ -6820,7 +6821,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
target_ulong address,
/* Check that the starting level is valid. */
ok = check_s2_startlevel(cpu, va_size == 64, level,
- inputsize, stride);
+ inputsize, stride, pamax);
if (!ok) {
/* AArch64 reports these as level 0 faults.
* AArch32 reports these as level 1 faults.
--
1.9.1
Re: [Qemu-devel] [PATCH v2 2/4] blockdev: Fix 'change' for slot devices
On Wed 20 Jan 2016 07:29:19 PM CET, Max Reitz wrote:
> @@ -2424,6 +2442,15 @@ static void qmp_blockdev_insert_anon_medium(const char
> *device,
>
> blk_insert_bs(blk, bs);
>
> +if (!blk_dev_has_tray(blk)) {
> +/* For tray-less devices, blockdev-close-tray is a no-op (or may not
> be
> + * called at all); therefore, the medium needs to be pushed into the
> + * slot here.
> + * Do it after blk_insert_bs() so blk_is_inserted(blk) returns the
> @load
> + * value passed here (i.e. true). */
> +blk_dev_change_media_cb(blk, true);
> +}
> +
> QTAILQ_INSERT_TAIL(&bdrv_states, bs, device_list);
> }
Any reason why you do this before updating bdrv_states ?
If the device has a tray this would happen afterwards, in
qmp_blockdev_close_tray().
Berto
Re: [Qemu-devel] [PATCH] linux-user: add option to intercept execve() syscalls
>> diff --git a/linux-user/main.c b/linux-user/main.c
>> index ee12035..5951279 100644
>> --- a/linux-user/main.c
>> +++ b/linux-user/main.c
>> @@ -79,6 +79,7 @@ static void usage(int exitcode);
>>
>> static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
>> const char *qemu_uname_release;
>> +const char *qemu_execve_path;
>>
>> /* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
>> we allocate a bigger stack. Need a better solution, for example
>> @@ -3828,6 +3829,11 @@ static void handle_arg_guest_base(const char *arg)
>> have_guest_base = 1;
>> }
>>
>> +static void handle_arg_execve(const char *arg)
>> +{
>> +qemu_execve_path = strdup(arg);
>
> I think you can use the parameter just as an on/off switch and
> realpath(argv[0]) as qemu_execve_path.
>
> I don't see any reason to use other binary than the one in use.
This was my initial approach too, but argv[0] can be just the filename
like "qemu-arm-static". And while I could add extra logic to look this
up in the PATH, someone could run it from a completely different
location. Then I looked for a way to get the path of the current
executable but every platform has its own way of doing that and I
didn't want to add all these cases.
https://stackoverflow.com/questions/1023306/finding-current-executables-path-without-proc-self-exe
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 0cbace4..d0b5442 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -5854,6 +5854,109 @@ static target_timer_t get_timer_id(abi_long arg)
>> return timerid;
>> }
>>
>> +#define BINPRM_BUF_SIZE 128
>
> This is defined in
Got it, I'll add this header and remove the definition.
>
>> +/* qemu_execve() Must return target values and target errnos. */
>> +static abi_long qemu_execve(char *filename, char *argv[],
>> + char *envp[])
>> +{
>> +char *i_arg = NULL, *i_name = NULL;
>> +char **new_argp;
>> +int argc, fd, ret, i, offset = 3;
>> +char *cp;
>> +char buf[BINPRM_BUF_SIZE];
>> +
>> +for (argc = 0; argv[argc] != NULL; argc++) {
>> +/* nothing */ ;
>> +}
>> +
>> +fd = open(filename, O_RDONLY);
>> +if (fd == -1) {
>> +return -ENOENT;
>
> return -errno; ?
Will fix in v2
>> +ret = read(fd, buf, BINPRM_BUF_SIZE);
>> +if (ret == -1) {
>> +close(fd);
>> +return -ENOENT;
>
> return -errno; ?
Will fix in v2
>> +}
>> +
>> +close(fd);
>> +
>> +/* adapted from the kernel
>> + *
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/binfmt_script.c
>> + */
>> +if ((buf[0] == '#') && (buf[1] == '!')) {
>
> what happens if read() < 2 ?
Hm, the easy option is for qemu_execve to return ENOEXEC or EIO.
Otherwise I could retry the read N times? I'm not sure how to handle
this if we don't want to return an error.
>> +/* Copy the original arguments with offset */
>> +for (i = 0; i < argc; i++) {
>> +new_argp[i + offset] = argv[i];
>> +}
>> +
>> +new_argp[0] = strdup(qemu_execve_path);
>> +new_argp[1] = strdup("-0");
>> +new_argp[offset] = filename;
>> +new_argp[argc + offset] = NULL;
>> +
>> +if (i_name) {
>> +new_argp[2] = i_name;
>> +new_argp[3] = i_name;
>> +
>> +if (i_arg) {
>> +new_argp[4] = i_arg;
>> +}
>> +} else {
>> +new_argp[2] = argv[0];
>> +}
>> +
>> +return get_errno(execve(qemu_execve_path, new_argp, envp));
>
> duplicate get_errno() with the caller.
I'll add the logic proposed bellow in this function and remove the
duplicate get_errno() from the caller.
>> /* do_syscall() should always have a single exit point at the end so
>> that actions, such as logging of syscall results, can be performed.
>> All errnos that do_syscall() returns must be -TARGET_. */
>> @@ -6113,7 +6216,13 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
>> arg1,
>>
>> if (!(p = lock_user_string(arg1)))
>> goto execve_efault;
>> -ret = get_errno(execve(p, argp, envp));
>> +
>> +if (qemu_execve_path && *qemu_execve_path) {
>> +ret = get_errno(qemu_execve(p, argp, envp));
>> +} else {
>> +ret = get_errno(execve(p, argp, envp));
>> +}
>> +
>
> what do you think of:
>
> ret = qemu_execve(p, argp, envp);
>
> and in qemu_execve():
>
> if (qemu_execve_path == NULL || *qemu_execve_path == 0) {
> return get_errno(execve(p, argp, envp));
> }
>
> so all the logic is in the function.
Sounds good, I'll include this in v2
Since I'm new to this style of contribution I have a couple of
questions. Is it ok that I deleted part of the patch for my reply to
code review, or should I have replied inline without deleting
anything? Should I send v2 after we resolve all the issues here or
should I send a v2 with proposed fixes but lacking the ones pending
replies?
Re: [Qemu-devel] [PATCH] dimm: Correct type of MemoryHotplugState->base
On Thu, 21 Jan 2016 12:37:51 +1100
David Gibson wrote:
> The 'base' field of MemoryHotplugState is ram_addr_t, which indicates that
> it exists in the abstract address space of RAM regions.
>
> However, the actual usage of this field indicates that it is a concrete
> physical address (it's passed as an offset to memory_region_add_subgregion
> for example).
>
> So, correct its type to 'hwaddr'.
>
> Signed-off-by: David Gibson
> ---
> include/hw/mem/pc-dimm.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index d83bf30..218dfb0 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -77,7 +77,7 @@ typedef struct PCDIMMDeviceClass {
> * @mr: hotplug memory address space container
> */
> typedef struct MemoryHotplugState {
> -ram_addr_t base;
> +hwaddr base;
> MemoryRegion mr;
> } MemoryHotplugState;
>
I agree with this fix but that's not the only place where
ram_addr_t needs to be replaced with hwaddr.
For example type of MachineState.[max]ram_size fields needs
to be changed as well. Because QEMU builds without CONFIG_XEN_BACKEND
on 32-bit hosts are broken since ram_addr_t is 32-bits there
while some targets assume and use it as 64-bit one.
Re: [Qemu-devel] [PATCH v3] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties
On Fri, Jan 22, 2016 at 12:26:52PM +0300, Valentin Rakush wrote:
> This patch adds support for qom-type-prop-list command to list object
> class properties. A later patch will use this functionality to
> implement x86_64-cpu properties.
>
> Signed-off-by: Valentin Rakush
> Cc: Luiz Capitulino
> Cc: Eric Blake
> Cc: Markus Armbruster
> Cc: Andreas Färber
> Cc: Daniel P. Berrange
> ---
> V2: Fixes after first review
> - changed command name from qom-type-list to qom-type-prop-list
> - changed memory allocation from g_malloc0 to g_new0
> - changed parameter name from path to typename
> - fixed wordings and comments
> - fixed source code formatting
> - registered the command in monitor
> V3: commit message fix
> - commit message changed to reflect actual command name
>
>
> hmp-commands.hx | 13 +
> hmp.c| 26 ++
> hmp.h| 1 +
> include/qom/object.h | 31 +++
> qapi-schema.json | 19 +++
> qmp-commands.hx | 6 ++
> qmp.c| 32
> qom/object.c | 7 +++
> 8 files changed, 135 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index bb52e4d..0aca653 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1734,6 +1734,19 @@ Print QOM properties of object at location @var{path}
> ETEXI
>
> {
> +.name = "qom-type-prop-list",
> +.args_type = "typename:s?",
The typename should be mandatory so remove the '?' here, then.
> +.params = "typename",
> +.help = "list QOM class properties",
> +.mhandler.cmd = hmp_qom_type_prop_list,
> +},
> +
> +STEXI
> +@item qom-type-prop-list [@var{typename}]
> +Print QOM properties of the type @var{typename}
> +ETEXI
> +
> +{
> .name = "qom-set",
> .args_type = "path:s,property:s,value:s",
> .params = "path property value",
> diff --git a/hmp.c b/hmp.c
> index 54f2620..4bad6a1 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -2052,6 +2052,32 @@ void hmp_qom_list(Monitor *mon, const QDict *qdict)
> hmp_handle_error(mon, &err);
> }
>
> +void hmp_qom_type_prop_list(Monitor *mon, const QDict *qdict)
> +{
> +const char *path = qdict_get_try_str(qdict, "path");
> +ObjectPropertyInfoList *list;
> +Error *err = NULL;
> +
> +if (!path) {
> +monitor_printf(mon, "/\n");
> +return;
> +}
...you don't need this check for path being NULL, you can assume
it is non-NULL
> +
> +list = qmp_qom_type_prop_list(path, &err);
> +if (!err) {
> +ObjectPropertyInfoList *start = list;
> +while (list) {
> +ObjectPropertyInfo *value = list->value;
> +
> +monitor_printf(mon, "%s (%s)\n",
> + value->name, value->type);
> +list = list->next;
> +}
> +qapi_free_ObjectPropertyInfoList(start);
> +}
> +hmp_handle_error(mon, &err);
> +}
> +
Aside from that I think this change looks good.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH RFC 0/7] Netfilter: Add each netdev a default filter
On Fri, Jan 22, 2016 at 04:36:44PM +0800, zhanghailiang wrote: > This series is a prerequisite for COLO, here we add each netdev > a default buffer filter, it is disabled by default, and has > no side effect for delivering packets in net layer. Why can't whatever is launching QEMU just setup filters explicitly if they want to use COLO ? I'm not seeing an obvious compelling reason to add this by default and then add extra code to deal with special casing its behaviour. > > Besides, patch 1 fixes the ouput information of 'info network' command > for filter. > > zhanghailiang (7): > net/filter: Fix the output information for command 'info network' > net/filter: Add a 'status' property for filter object > net/filter: Skip the disabled filter when delivering packets > net/filter: Introduce a helper to add a filter to the netdev > filter-buffer: Accept zero interval > net/filter: Add a default filter to each netdev > net/filter: prevent the default filter to be deleted > > include/net/filter.h | 25 +++- > net/dump.c | 2 - > net/filter-buffer.c | 10 > net/filter.c | 163 > +-- > net/net.c| 27 - > 5 files changed, 194 insertions(+), 33 deletions(-) Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v1] kvm/x86: Hyper-V tsc page setup
On 24/12/2015 10:33, Andrey Smetanin wrote:
> Lately tsc page was implemented but filled with empty
> values. This patch setup tsc page scale and offset based
> on vcpu tsc, tsc_khz and HV_X64_MSR_TIME_REF_COUNT value.
>
> The valid tsc page drops HV_X64_MSR_TIME_REF_COUNT msr
> reads count to zero which potentially improves performance.
>
> The patch applies on top of
> 'kvm: Make vcpu->requests as 64 bit bitmap'
> previously sent.
>
> Signed-off-by: Andrey Smetanin
> CC: Paolo Bonzini
> CC: Gleb Natapov
> CC: Roman Kagan
> CC: Denis V. Lunev
> CC: qemu-devel@nongnu.org
Actually there are some more issues:
- unless KVM can use a master clock, it is incorrect to set up the TSC
page this way; the sequence needs to be 0x in that case
- writing the TSC page must be done while all VCPUs are stopped, because
the TSC page doesn't provide the possibility for the guest to retry in
the middle of an update (like seqcount in Linux doess)
In the end, the TSC page is actually pretty similar to the kvmclock
master clock and it makes sense to build it on the master clock too.
I'll post a patch next week.
Paolo
> ---
> arch/x86/kvm/hyperv.c| 117
> +--
> arch/x86/kvm/hyperv.h| 2 +
> arch/x86/kvm/x86.c | 12 +
> include/linux/kvm_host.h | 1 +
> 4 files changed, 117 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index d50675a..504fdc7 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -753,6 +753,105 @@ static int kvm_hv_msr_set_crash_data(struct kvm_vcpu
> *vcpu,
> return 0;
> }
>
> +static u64 calc_tsc_page_scale(u32 tsc_khz)
> +{
> + /*
> + * reftime (in 100ns) = tsc * tsc_scale / 2^64 + tsc_offset
> + * so reftime_delta = (tsc_delta * tsc_scale) / 2^64
> + * so tsc_scale = (2^64 * reftime_delta)/tsc_delta
> + * so tsc_scale = (2^64 * 10 * 10^6) / tsc_hz = (2^64 * 1) / tsc_khz
> + * so tsc_scale = (2^63 * 2 * 1) / tsc_khz
> + */
> + return mul_u64_u32_div(1ULL << 63, 2 * 1, tsc_khz);
> +}
> +
> +static int write_tsc_page(struct kvm *kvm, u64 gfn,
> + PHV_REFERENCE_TSC_PAGE tsc_ref)
> +{
> + if (kvm_write_guest(kvm, gfn_to_gpa(gfn),
> + tsc_ref, sizeof(*tsc_ref)))
> + return 1;
> + mark_page_dirty(kvm, gfn);
> + return 0;
> +}
> +
> +static int read_tsc_page(struct kvm *kvm, u64 gfn,
> + PHV_REFERENCE_TSC_PAGE tsc_ref)
> +{
> + if (kvm_read_guest(kvm, gfn_to_gpa(gfn),
> +tsc_ref, sizeof(*tsc_ref)))
> + return 1;
> + return 0;
> +}
> +
> +static u64 calc_tsc_page_time(struct kvm_vcpu *vcpu,
> + PHV_REFERENCE_TSC_PAGE tsc_ref)
> +{
> +
> + u64 tsc = kvm_read_l1_tsc(vcpu, rdtsc());
> +
> + return mul_u64_u64_shr(tsc, tsc_ref->tsc_scale, 64)
> + + tsc_ref->tsc_offset;
> +}
> +
> +static int setup_blank_tsc_page(struct kvm_vcpu *vcpu, u64 gfn)
> +{
> + HV_REFERENCE_TSC_PAGE tsc_ref;
> +
> + memset(&tsc_ref, 0, sizeof(tsc_ref));
> + return write_tsc_page(vcpu->kvm, gfn, &tsc_ref);
> +}
> +
> +int kvm_hv_setup_tsc_page(struct kvm_vcpu *vcpu)
> +{
> + struct kvm *kvm = vcpu->kvm;
> + struct kvm_hv *hv = &kvm->arch.hyperv;
> + HV_REFERENCE_TSC_PAGE tsc_ref;
> + u32 tsc_khz;
> + int r;
> + u64 gfn, ref_time, tsc_scale, tsc_offset, tsc;
> +
> + if (WARN_ON_ONCE(!(hv->hv_tsc_page & HV_X64_MSR_TSC_REFERENCE_ENABLE)))
> + return -EINVAL;
> +
> + gfn = hv->hv_tsc_page >> HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT;
> + vcpu_debug(vcpu, "tsc page gfn 0x%llx\n", gfn);
> +
> + tsc_khz = vcpu->arch.virtual_tsc_khz;
> + if (!tsc_khz) {
> + vcpu_unimpl(vcpu, "no tsc khz\n");
> + return setup_blank_tsc_page(vcpu, gfn);
> + }
> +
> + r = read_tsc_page(kvm, gfn, &tsc_ref);
> + if (r) {
> + vcpu_err(vcpu, "can't access tsc page gfn 0x%llx\n", gfn);
> + return r;
> + }
> +
> + tsc_scale = calc_tsc_page_scale(tsc_khz);
> + ref_time = get_time_ref_counter(kvm);
> + tsc = kvm_read_l1_tsc(vcpu, rdtsc());
> +
> + /* tsc_offset = reftime - tsc * tsc_scale / 2^64 */
> + tsc_offset = ref_time - mul_u64_u64_shr(tsc, tsc_scale, 64);
> + vcpu_debug(vcpu, "tsc khz %u tsc %llu scale %llu offset %llu\n",
> +tsc_khz, tsc, tsc_scale, tsc_offset);
> +
> + tsc_ref.tsc_sequence++;
> + if (tsc_ref.tsc_sequence == 0)
> + tsc_ref.tsc_sequence = 1;
> +
> + tsc_ref.tsc_scale = tsc_scale;
> + tsc_ref.tsc_offset = tsc_offset;
> +
> + vcpu_debug(vcpu, "tsc page calibration time %llu vs. reftime %llu\n",
> +calc_tsc_page_time(vcpu, &tsc_ref),
> +get_time_ref_counter(kvm));
> +
> + return write_tsc_page(kvm, gfn, &tsc_ref);
> +}
> +
> static in
Re: [Qemu-devel] [PATCH v1] kvm/x86: Hyper-V tsc page setup
On 01/22/2016 01:08 PM, Paolo Bonzini wrote:
On 24/12/2015 10:33, Andrey Smetanin wrote:
Lately tsc page was implemented but filled with empty
values. This patch setup tsc page scale and offset based
on vcpu tsc, tsc_khz and HV_X64_MSR_TIME_REF_COUNT value.
The valid tsc page drops HV_X64_MSR_TIME_REF_COUNT msr
reads count to zero which potentially improves performance.
The patch applies on top of
'kvm: Make vcpu->requests as 64 bit bitmap'
previously sent.
Signed-off-by: Andrey Smetanin
CC: Paolo Bonzini
CC: Gleb Natapov
CC: Roman Kagan
CC: Denis V. Lunev
CC: qemu-devel@nongnu.org
Actually there are some more issues:
- unless KVM can use a master clock, it is incorrect to set up the TSC
page this way; the sequence needs to be 0x in that case
0x is not an invalid value for tsc page,
see https://lkml.org/lkml/2015/11/2/655
- writing the TSC page must be done while all VCPUs are stopped, because
the TSC page doesn't provide the possibility for the guest to retry in
the middle of an update (like seqcount in Linux doess)
I think Windows guest gives tsc page address at boot time and protects
against other vcpu's tsc page access.
In the end, the TSC page is actually pretty similar to the kvmclock
master clock and it makes sense to build it on the master clock too.
I'll post a patch next week.
Paolo
---
arch/x86/kvm/hyperv.c| 117 +--
arch/x86/kvm/hyperv.h| 2 +
arch/x86/kvm/x86.c | 12 +
include/linux/kvm_host.h | 1 +
4 files changed, 117 insertions(+), 15 deletions(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index d50675a..504fdc7 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -753,6 +753,105 @@ static int kvm_hv_msr_set_crash_data(struct kvm_vcpu
*vcpu,
return 0;
}
+static u64 calc_tsc_page_scale(u32 tsc_khz)
+{
+ /*
+* reftime (in 100ns) = tsc * tsc_scale / 2^64 + tsc_offset
+* so reftime_delta = (tsc_delta * tsc_scale) / 2^64
+* so tsc_scale = (2^64 * reftime_delta)/tsc_delta
+* so tsc_scale = (2^64 * 10 * 10^6) / tsc_hz = (2^64 * 1) / tsc_khz
+* so tsc_scale = (2^63 * 2 * 1) / tsc_khz
+*/
+ return mul_u64_u32_div(1ULL << 63, 2 * 1, tsc_khz);
+}
+
+static int write_tsc_page(struct kvm *kvm, u64 gfn,
+ PHV_REFERENCE_TSC_PAGE tsc_ref)
+{
+ if (kvm_write_guest(kvm, gfn_to_gpa(gfn),
+ tsc_ref, sizeof(*tsc_ref)))
+ return 1;
+ mark_page_dirty(kvm, gfn);
+ return 0;
+}
+
+static int read_tsc_page(struct kvm *kvm, u64 gfn,
+PHV_REFERENCE_TSC_PAGE tsc_ref)
+{
+ if (kvm_read_guest(kvm, gfn_to_gpa(gfn),
+ tsc_ref, sizeof(*tsc_ref)))
+ return 1;
+ return 0;
+}
+
+static u64 calc_tsc_page_time(struct kvm_vcpu *vcpu,
+ PHV_REFERENCE_TSC_PAGE tsc_ref)
+{
+
+ u64 tsc = kvm_read_l1_tsc(vcpu, rdtsc());
+
+ return mul_u64_u64_shr(tsc, tsc_ref->tsc_scale, 64)
+ + tsc_ref->tsc_offset;
+}
+
+static int setup_blank_tsc_page(struct kvm_vcpu *vcpu, u64 gfn)
+{
+ HV_REFERENCE_TSC_PAGE tsc_ref;
+
+ memset(&tsc_ref, 0, sizeof(tsc_ref));
+ return write_tsc_page(vcpu->kvm, gfn, &tsc_ref);
+}
+
+int kvm_hv_setup_tsc_page(struct kvm_vcpu *vcpu)
+{
+ struct kvm *kvm = vcpu->kvm;
+ struct kvm_hv *hv = &kvm->arch.hyperv;
+ HV_REFERENCE_TSC_PAGE tsc_ref;
+ u32 tsc_khz;
+ int r;
+ u64 gfn, ref_time, tsc_scale, tsc_offset, tsc;
+
+ if (WARN_ON_ONCE(!(hv->hv_tsc_page & HV_X64_MSR_TSC_REFERENCE_ENABLE)))
+ return -EINVAL;
+
+ gfn = hv->hv_tsc_page >> HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT;
+ vcpu_debug(vcpu, "tsc page gfn 0x%llx\n", gfn);
+
+ tsc_khz = vcpu->arch.virtual_tsc_khz;
+ if (!tsc_khz) {
+ vcpu_unimpl(vcpu, "no tsc khz\n");
+ return setup_blank_tsc_page(vcpu, gfn);
+ }
+
+ r = read_tsc_page(kvm, gfn, &tsc_ref);
+ if (r) {
+ vcpu_err(vcpu, "can't access tsc page gfn 0x%llx\n", gfn);
+ return r;
+ }
+
+ tsc_scale = calc_tsc_page_scale(tsc_khz);
+ ref_time = get_time_ref_counter(kvm);
+ tsc = kvm_read_l1_tsc(vcpu, rdtsc());
+
+ /* tsc_offset = reftime - tsc * tsc_scale / 2^64 */
+ tsc_offset = ref_time - mul_u64_u64_shr(tsc, tsc_scale, 64);
+ vcpu_debug(vcpu, "tsc khz %u tsc %llu scale %llu offset %llu\n",
+ tsc_khz, tsc, tsc_scale, tsc_offset);
+
+ tsc_ref.tsc_sequence++;
+ if (tsc_ref.tsc_sequence == 0)
+ tsc_ref.tsc_sequence = 1;
+
+ tsc_ref.tsc_scale = tsc_scale;
+ tsc_ref.tsc_offset = tsc_offset;
+
+ vcpu_debug(vcpu, "tsc page calibration time %llu vs. reftime %llu\n",
+ calc_tsc_page_time(vcpu, &tsc_ref),
Re: [Qemu-devel] [PATCH v2 3/3] target-arm: Implement the S2 MMU inputsize > pamax check
Edgar E. Iglesias writes:
> From: "Edgar E. Iglesias"
>
> Implement the inputsize > pamax check for Stage 2 translations.
> We have multiple choices for how to respond to errors and
> choose to fault.
>
> Signed-off-by: Edgar E. Iglesias
> ---
> target-arm/helper.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 4abeb4d..9a7ff5e 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -6808,7 +6808,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> target_ulong address,
> */
> int startlevel = extract32(tcr->raw_tcr, 6, 2);
> unsigned int pamax = arm_pamax(cpu);
> -bool ok;
> +bool ok = true;
>
> if (va_size == 32 || stride == 9) {
> /* AArch32 or 4KB pages */
> @@ -6818,9 +6818,17 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> target_ulong address,
> level = 3 - startlevel;
> }
>
> -/* Check that the starting level is valid. */
> -ok = check_s2_startlevel(cpu, va_size == 64, level,
> - inputsize, stride, pamax);
> +if (va_size == 64 &&
> +inputsize > pamax &&
> +(arm_el_is_aa64(env, 1) || inputsize > 40)) {
If va_size == 64 doesn't that imply arm_el_is_aa64(env, 1)? Looking
further up the function it seems that is what sets va_size in the first
place. I think that makes the inputsize > 40 check redundant.
> +/* We have multiple choices but choose to fault. */
> +ok = false;
> +}
> +if (ok) {
> +/* Check that the starting level is valid. */
> +ok = check_s2_startlevel(cpu, va_size == 64, level,
> + inputsize, stride, pamax);
> +}
> if (!ok) {
> /* AArch64 reports these as level 0 faults.
> * AArch32 reports these as level 1 faults.
I'm not a fan of the ok = true / ok = false / ok =
check_s2_start_level() / if (!ok) ping-pong here as it is hard to
follow. I'm not sure how you could make it cleaner to follow though.
Maybe something like:
/* For stage 2 translations the starting level is specified by the
* VTCR_EL2.SL0 field (whose interpretation depends on the page size)
*/
int startlevel = extract32(tcr->raw_tcr, 6, 2);
unsigned int pamax = arm_pamax(cpu);
bool is_aarch64_regime = (va_size == 64);
bool ok;
if (va_size == 32 || stride == 9) {
/* AArch32 or 4KB pages */
level = 2 - startlevel;
} else {
/* 16KB or 64KB pages */
level = 3 - startlevel;
}
if (is_aarch64_regime &&
inputsize > pamax) {
/* We have multiple choices but choose to fault. */
ok = false;
} else {
/* Check that the starting level is valid. */
ok = check_s2_startlevel(cpu, is_aarch64_regime, level,
inputsize, stride, pamax);
}
if (!ok) {
/* AArch64 reports these as level 0 faults.
* AArch32 reports these as level 1 faults.
*/
level = is_aarch64_regime ? 0 : 1;
fault_type = translation_fault;
goto do_fault;
}
But I'm wondering if it just makes more sense to push the:
is_aarch64_regime && inputsize > pamax
Check into check_s2_startlevel? Then you could just have a simple call
which succeeds or falls through to a fault?
/* Check that the starting level is valid. */
if (!check_s2_startlevel(cpu, is_aarch64_regime, level,
inputsize, stride, pamax) ){
/* AArch64 reports these as level 0 faults.
* AArch32 reports these as level 1 faults.
*/
level = is_aarch64_regime ? 0 : 1;
fault_type = translation_fault;
goto do_fault;
}
--
Alex Bennée
Re: [Qemu-devel] [PATCH] linux-user: add option to intercept execve() syscalls
Le 22/01/2016 11:01, Petros Angelatos a écrit :
>>> diff --git a/linux-user/main.c b/linux-user/main.c
>>> index ee12035..5951279 100644
>>> --- a/linux-user/main.c
>>> +++ b/linux-user/main.c
>>> @@ -79,6 +79,7 @@ static void usage(int exitcode);
>>>
>>> static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
>>> const char *qemu_uname_release;
>>> +const char *qemu_execve_path;
>>>
>>> /* XXX: on x86 MAP_GROWSDOWN only works if ESP <= address + 32, so
>>> we allocate a bigger stack. Need a better solution, for example
>>> @@ -3828,6 +3829,11 @@ static void handle_arg_guest_base(const char *arg)
>>> have_guest_base = 1;
>>> }
>>>
>>> +static void handle_arg_execve(const char *arg)
>>> +{
>>> +qemu_execve_path = strdup(arg);
>>
>> I think you can use the parameter just as an on/off switch and
>> realpath(argv[0]) as qemu_execve_path.
>>
>> I don't see any reason to use other binary than the one in use.
>
> This was my initial approach too, but argv[0] can be just the filename
> like "qemu-arm-static". And while I could add extra logic to look this
> up in the PATH, someone could run it from a completely different
> location. Then I looked for a way to get the path of the current
> executable but every platform has its own way of doing that and I
> didn't want to add all these cases.
>
> https://stackoverflow.com/questions/1023306/finding-current-executables-path-without-proc-self-exe
linux-user works only on linux.
qemu uses glib-2.0, so you can use g_find_program_in_path().
>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>> index 0cbace4..d0b5442 100644
>>> --- a/linux-user/syscall.c
>>> +++ b/linux-user/syscall.c
>>> @@ -5854,6 +5854,109 @@ static target_timer_t get_timer_id(abi_long arg)
>>> return timerid;
>>> }
>>>
>>> +#define BINPRM_BUF_SIZE 128
>>
>> This is defined in
>
> Got it, I'll add this header and remove the definition.
>
>>
>>> +/* qemu_execve() Must return target values and target errnos. */
>>> +static abi_long qemu_execve(char *filename, char *argv[],
>>> + char *envp[])
>>> +{
>>> +char *i_arg = NULL, *i_name = NULL;
>>> +char **new_argp;
>>> +int argc, fd, ret, i, offset = 3;
>>> +char *cp;
>>> +char buf[BINPRM_BUF_SIZE];
>>> +
>>> +for (argc = 0; argv[argc] != NULL; argc++) {
>>> +/* nothing */ ;
>>> +}
>>> +
>>> +fd = open(filename, O_RDONLY);
>>> +if (fd == -1) {
>>> +return -ENOENT;
>>
>> return -errno; ?
>
> Will fix in v2
>
>>> +ret = read(fd, buf, BINPRM_BUF_SIZE);
>>> +if (ret == -1) {
>>> +close(fd);
>>> +return -ENOENT;
>>
>> return -errno; ?
>
> Will fix in v2
>
>>> +}
>>> +
>>> +close(fd);
>>> +
>>> +/* adapted from the kernel
>>> + *
>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/fs/binfmt_script.c
>>> + */
>>> +if ((buf[0] == '#') && (buf[1] == '!')) {
>>
>> what happens if read() < 2 ?
>
> Hm, the easy option is for qemu_execve to return ENOEXEC or EIO.
> Otherwise I could retry the read N times? I'm not sure how to handle
> this if we don't want to return an error.
if we have less than 2 bytes, we can guess it is not executable...
>>> +/* Copy the original arguments with offset */
>>> +for (i = 0; i < argc; i++) {
>>> +new_argp[i + offset] = argv[i];
>>> +}
>>> +
>>> +new_argp[0] = strdup(qemu_execve_path);
>>> +new_argp[1] = strdup("-0");
>>> +new_argp[offset] = filename;
>>> +new_argp[argc + offset] = NULL;
>>> +
>>> +if (i_name) {
>>> +new_argp[2] = i_name;
>>> +new_argp[3] = i_name;
>>> +
>>> +if (i_arg) {
>>> +new_argp[4] = i_arg;
>>> +}
>>> +} else {
>>> +new_argp[2] = argv[0];
>>> +}
>>> +
>>> +return get_errno(execve(qemu_execve_path, new_argp, envp));
>>
>> duplicate get_errno() with the caller.
>
> I'll add the logic proposed bellow in this function and remove the
> duplicate get_errno() from the caller.
>
>>> /* do_syscall() should always have a single exit point at the end so
>>> that actions, such as logging of syscall results, can be performed.
>>> All errnos that do_syscall() returns must be -TARGET_. */
>>> @@ -6113,7 +6216,13 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
>>> arg1,
>>>
>>> if (!(p = lock_user_string(arg1)))
>>> goto execve_efault;
>>> -ret = get_errno(execve(p, argp, envp));
>>> +
>>> +if (qemu_execve_path && *qemu_execve_path) {
>>> +ret = get_errno(qemu_execve(p, argp, envp));
>>> +} else {
>>> +ret = get_errno(execve(p, argp, envp));
>>> +}
>>> +
>>
>> what do you think of:
>>
>> ret = qemu_execve(p, argp, envp);
>>
>> and in qemu_execve():
>>
>> if (qemu_execve_path == NULL || *qemu_execve_path == 0) {
>> return get_errno(execve(p, argp, envp));
>> }
>>
>> so all the logic is in the
Re: [Qemu-devel] [PATCH RFC 0/7] Netfilter: Add each netdev a default filter
On 2016/1/22 18:07, Daniel P. Berrange wrote: On Fri, Jan 22, 2016 at 04:36:44PM +0800, zhanghailiang wrote: This series is a prerequisite for COLO, here we add each netdev a default buffer filter, it is disabled by default, and has no side effect for delivering packets in net layer. Why can't whatever is launching QEMU just setup filters explicitly if they want to use COLO ? I'm not seeing an obvious compelling reason to add this by default and then add extra code to deal with special casing its behaviour. The main reason is, we hope to support hot add network during VM's COLO lifetime in the future. (I'm not quite sure if this usage case is really exist, but we don't want the VM in COLO state has too many limitations.) Maybe add an option that users can control if they want to use COLO or not is more acceptable ? With this option, we can decide whether to add the default filter or not. Or, we could dynamically add filter while users ask to go into COLO state for VM. (We have discussed this before in community, and Jason suggested me to add default filter for each netdev to support hot-add network during COLO state). What's your suggestion ? Thanks, Hailiang Besides, patch 1 fixes the ouput information of 'info network' command for filter. zhanghailiang (7): net/filter: Fix the output information for command 'info network' net/filter: Add a 'status' property for filter object net/filter: Skip the disabled filter when delivering packets net/filter: Introduce a helper to add a filter to the netdev filter-buffer: Accept zero interval net/filter: Add a default filter to each netdev net/filter: prevent the default filter to be deleted include/net/filter.h | 25 +++- net/dump.c | 2 - net/filter-buffer.c | 10 net/filter.c | 163 +-- net/net.c| 27 - 5 files changed, 194 insertions(+), 33 deletions(-) Regards, Daniel
Re: [Qemu-devel] [PATCH RFC 0/7] Netfilter: Add each netdev a default filter
On Fri, Jan 22, 2016 at 06:35:48PM +0800, Hailiang Zhang wrote: > On 2016/1/22 18:07, Daniel P. Berrange wrote: > >On Fri, Jan 22, 2016 at 04:36:44PM +0800, zhanghailiang wrote: > >>This series is a prerequisite for COLO, here we add each netdev > >>a default buffer filter, it is disabled by default, and has > >>no side effect for delivering packets in net layer. > > > >Why can't whatever is launching QEMU just setup filters explicitly > >if they want to use COLO ? I'm not seeing an obvious compelling > >reason to add this by default and then add extra code to deal > >with special casing its behaviour. > > > > The main reason is, we hope to support hot add network during VM's COLO > lifetime in the future. (I'm not quite sure if this usage case is really > exist, > but we don't want the VM in COLO state has too many limitations.) > > Maybe add an option that users can control if they want to use COLO or not is > more > acceptable ? With this option, we can decide whether to add the default > filter or not. > Or, we could dynamically add filter while users ask to go into COLO state for > VM. > (We have discussed this before in community, and Jason suggested me to add > default > filter for each netdev to support hot-add network during COLO state). > > What's your suggestion ? Why can't the app hot-adding the network interface also configure a filter at that time if they're using COLO ? Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [Minios-devel] [PATCH v8 0/] Begin to disentangle libxenctrl and provide some stable libraries
On Tue, 2016-01-19 at 15:44 +, Ian Campbell wrote:
> On Fri, 2016-01-15 at 13:22 +, Ian Campbell wrote:
> >
> > Therefore needing attention from Ian and/or Wei are:
> >
> > tools/libs/foreignmemory: Mention restrictions on fork in docs.
> > N tools/libs/evtchn: Use uint32_t for domid arguments
> > D tools/libs/gnttab: Extensive updates to API documentation.
> > tools/libs/call: linux: touch newly allocated pages after madvise
> > l
> > tools/libs/{call,evtchn}: Document requirements around forking.
> > Rtools/libs/*: Use O_CLOEXEC on Linux and FreeBSD
>
> Thanks to Wei for acking all of these. This set of series is now ready to
> go in, but we've not had a push for a little while and this is
> potentially
> disruptive so I'm going to hold off for now until we get a push.
We've now had a push in 78610 so I'm going to go ahead with applying this
mass of patches today.
> There are one or two patches which will require rebasing over Jeurgens
> introduction of tools/helpers, I'll resend just those ones though (or at
> least only the Xen part of this series).
Ian.
Re: [Qemu-devel] [PATCH] linux-user: add option to intercept execve() syscalls
On 22 January 2016 at 10:33, Laurent Vivier wrote: > Le 22/01/2016 11:01, Petros Angelatos a écrit : >> This was my initial approach too, but argv[0] can be just the filename >> like "qemu-arm-static". And while I could add extra logic to look this >> up in the PATH, someone could run it from a completely different >> location. Then I looked for a way to get the path of the current >> executable but every platform has its own way of doing that and I >> didn't want to add all these cases. >> >> https://stackoverflow.com/questions/1023306/finding-current-executables-path-without-proc-self-exe > > linux-user works only on linux. > qemu uses glib-2.0, so you can use g_find_program_in_path(). If QEMU was started via execle() to set the environment of the executed process and that specified environment has a different PATH, then g_find_program_in_path() will give the wrong answer. Using AT_EXECFN (perhaps with a fallback to /proc/self/exe) seems like a better approach to me. >> questions. Is it ok that I deleted part of the patch for my reply to >> code review, or should I have replied inline without deleting > > Generally, it's better to not delete parts. So, someone tacking the mail > thread at a moment can read the whole history in the last mail. I tend to happily delete parts and assume that readers have access to the thread (via the archive or in their mail readers). Not deleting bits makes it hard to read replies if there's a conversation about a small part of a large patch. thanks -- PMM
Re: [Qemu-devel] [PATCH v2 4/9] ipmi: introduce a struct ipmi_sdr_compact
On Thu, 21 Jan 2016 18:18:49 +0100
Cédric Le Goater wrote:
> Currently, sdr attributes are identified using byte offsets and this
> can be a bit confusing.
>
> This patch adds a struct ipmi_sdr_compact conforming to the IPMI specs
> and replaces byte offsets with names. It also introduces and uses a
> struct ipmi_sdr_header in sections of the code where no assumption is
> made on the type of SDR. This leave rooms to potential usage of other
> types in the future.
>
Turning all these magic numbers into understandable names is definitely a
great idea !
See comments below.
> Signed-off-by: Cédric Le Goater
> ---
> hw/ipmi/ipmi_bmc_sim.c | 65
> +++---
> include/hw/ipmi/ipmi.h | 44 ++
> 2 files changed, 84 insertions(+), 25 deletions(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index fc596a548df7..31f990199154 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -323,11 +323,15 @@ static void sdr_inc_reservation(IPMISdr *sdr)
> static int sdr_add_entry(IPMIBmcSim *ibs, const uint8_t *entry,
> unsigned int len, uint16_t *recid)
> {
> +struct ipmi_sdr_header *sdrh_entry = (struct ipmi_sdr_header *) entry;
This looks like the entry argument should be struct ipmi_sdr_header * and
you would not need sdrh_entry.
> +struct ipmi_sdr_header *sdrh =
> +(struct ipmi_sdr_header *) &ibs->sdr.sdr[ibs->sdr.next_free];
> +
> if ((len < 5) || (len > 255)) {
> return 1;
> }
>
> -if (entry[4] != len - 5) {
> +if (sdrh_entry->rec_length != len - 5) {
> return 1;
> }
>
> @@ -336,10 +340,10 @@ static int sdr_add_entry(IPMIBmcSim *ibs, const uint8_t
> *entry,
> return 1;
> }
>
> -memcpy(ibs->sdr.sdr + ibs->sdr.next_free, entry, len);
> -ibs->sdr.sdr[ibs->sdr.next_free] = ibs->sdr.next_rec_id & 0xff;
> -ibs->sdr.sdr[ibs->sdr.next_free+1] = (ibs->sdr.next_rec_id >> 8) & 0xff;
> -ibs->sdr.sdr[ibs->sdr.next_free+2] = 0x51; /* Conform to IPMI 1.5 spec */
> +memcpy(sdrh, entry, len);
> +sdrh->rec_id[0] = ibs->sdr.next_rec_id & 0xff;
> +sdrh->rec_id[1] = (ibs->sdr.next_rec_id >> 8) & 0xff;
> +sdrh->sdr_version = 0x51; /* Conform to IPMI 1.5 spec */
>
> if (recid) {
> *recid = ibs->sdr.next_rec_id;
> @@ -357,8 +361,10 @@ static int sdr_find_entry(IPMISdr *sdr, uint16_t recid,
> unsigned int pos = *retpos;
>
> while (pos < sdr->next_free) {
> -uint16_t trec = sdr->sdr[pos] | (sdr->sdr[pos + 1] << 8);
> -unsigned int nextpos = pos + sdr->sdr[pos + 4];
> +struct ipmi_sdr_header *sdrh =
> +(struct ipmi_sdr_header *) &sdr->sdr[pos];
> +uint16_t trec = ipmi_sdr_recid(sdrh);
> +unsigned int nextpos = pos + sdrh->rec_length;
>
> if (trec == recid) {
> if (nextrec) {
> @@ -507,29 +513,32 @@ static void ipmi_init_sensors_from_sdrs(IPMIBmcSim *s)
>
> pos = 0;
> for (i = 0; !sdr_find_entry(&s->sdr, i, &pos, NULL); i++) {
> -uint8_t *sdr = s->sdr.sdr + pos;
> -unsigned int len = sdr[4];
> +struct ipmi_sdr_compact *sdr =
> +(struct ipmi_sdr_compact *) &s->sdr.sdr[pos];
> +unsigned int len = sdr->header.rec_length;
>
> if (len < 20) {
> continue;
> }
> -if ((sdr[3] < 1) || (sdr[3] > 2)) {
> +if (sdr->header.rec_type != IPMI_SDR_COMPACT_TYPE) {
> continue; /* Not a sensor SDR we set from */
> }
>
> -if (sdr[7] > MAX_SENSORS) {
> +if (sdr->sensor_owner_number > MAX_SENSORS) {
> continue;
> }
> -sens = s->sensors + sdr[7];
> +sens = s->sensors + sdr->sensor_owner_number;
>
> IPMI_SENSOR_SET_PRESENT(sens, 1);
> -IPMI_SENSOR_SET_SCAN_ON(sens, (sdr[10] >> 6) & 1);
> -IPMI_SENSOR_SET_EVENTS_ON(sens, (sdr[10] >> 5) & 1);
> -sens->assert_suppt = sdr[14] | (sdr[15] << 8);
> -sens->deassert_suppt = sdr[16] | (sdr[17] << 8);
> -sens->states_suppt = sdr[18] | (sdr[19] << 8);
> -sens->sensor_type = sdr[12];
> -sens->evt_reading_type_code = sdr[13] & 0x7f;
> +IPMI_SENSOR_SET_SCAN_ON(sens, (sdr->sensor_init >> 6) & 1);
> +IPMI_SENSOR_SET_EVENTS_ON(sens, (sdr->sensor_init >> 5) & 1);
> +sens->assert_suppt = sdr->assert_mask[0] | (sdr->assert_mask[1] <<
> 8);
> +sens->deassert_suppt =
> +sdr->deassert_mask[0] | (sdr->deassert_mask[1] << 8);
> +sens->states_suppt =
> +sdr->discrete_mask[0] | (sdr->discrete_mask[1] << 8);
> +sens->sensor_type = sdr->sensor_type;
> +sens->evt_reading_type_code = sdr->reading_type & 0x7f;
>
> /* Enable all the events that are supported. */
> sens->assert_enable = sens->assert_suppt;
> @@ -1155,6 +1164,7 @@ static void get_sdr(IPMIBmcSim *i
Re: [Qemu-devel] [PATCH v2] pc: allow raising low memory via max-ram-below-4g option
Hi, > > > I wonder whether we should just bite the bullet and ask management to > > > maintain the physical memory map for us, instead of trying to give us > > > hints. > > > > I doubt this simplified things, given the backward compatibility > > constrains we have. > > > > cheers, > > Gerd > > That's exactly what would become simple. > For backwards compatibility we would leave things alone > if the new flags for the memory map aren't specified. But we'll add a bunch of new code for the new config mode which allows management to maintain the physical memory map. And we'll expect management know about a bunch of machine type internals. That isn't a simplification. > This would allow people to e.g. allocate phy address > ranges for things like nvdimm which has been > problematic in the past. Didn't follow nvdimm discussions. If you think we really need that anyway to solve certain issues, sure, go ahead and I happily adjust this patch to use the new infrastructure. cheers, Gerd
Re: [Qemu-devel] [PATCH v2 5/9] ipmi: fix SDR length value
On Thu, 21 Jan 2016 18:18:50 +0100
Cédric Le Goater wrote:
> The IPMI BMC simulator populates the SDR table with a set of initial
> SDRs. The length of each SDR is taken from the record itself (byte 4)
> which does not include the size of the header. But, the full length
> (header + data) is required by the sdr_add_entry() routine.
>
> Signed-off-by: Cédric Le Goater
The patch is good but IMHO it should come before patch 4 because this is bugfix
that could be applied right away, while patch 4 is code cleanup that may need
some more discussion.
> ---
> hw/ipmi/ipmi_bmc_sim.c | 18 +-
> include/hw/ipmi/ipmi.h | 1 +
> 2 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 31f990199154..803c7e5130c0 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -327,11 +327,11 @@ static int sdr_add_entry(IPMIBmcSim *ibs, const uint8_t
> *entry,
> struct ipmi_sdr_header *sdrh =
> (struct ipmi_sdr_header *) &ibs->sdr.sdr[ibs->sdr.next_free];
>
> -if ((len < 5) || (len > 255)) {
> +if ((len < IPMI_SDR_HEADER_SIZE) || (len > 255)) {
> return 1;
> }
>
> -if (sdrh_entry->rec_length != len - 5) {
> +if (ipmi_sdr_length(sdrh_entry) != len) {
> return 1;
> }
>
> @@ -364,7 +364,7 @@ static int sdr_find_entry(IPMISdr *sdr, uint16_t recid,
> struct ipmi_sdr_header *sdrh =
> (struct ipmi_sdr_header *) &sdr->sdr[pos];
> uint16_t trec = ipmi_sdr_recid(sdrh);
> -unsigned int nextpos = pos + sdrh->rec_length;
> +unsigned int nextpos = pos + ipmi_sdr_length(sdrh);
>
> if (trec == recid) {
> if (nextrec) {
> @@ -1179,7 +1179,7 @@ static void get_sdr(IPMIBmcSim *ibs,
>
> sdrh = (struct ipmi_sdr_header *) &ibs->sdr.sdr[pos];
>
> -if (cmd[6] > sdrh->rec_length) {
> +if (cmd[6] > ipmi_sdr_length(sdrh)) {
> rsp[2] = IPMI_CC_PARM_OUT_OF_RANGE;
> return;
> }
> @@ -1188,7 +1188,7 @@ static void get_sdr(IPMIBmcSim *ibs,
> IPMI_ADD_RSP_DATA((nextrec >> 8) & 0xff);
>
> if (cmd[7] == 0xff) {
> -cmd[7] = sdrh->rec_length - cmd[6];
> +cmd[7] = ipmi_sdr_length(sdrh) - cmd[6];
> }
>
> if ((cmd[7] + *rsp_len) > max_rsp_len) {
> @@ -1659,22 +1659,22 @@ static void ipmi_sim_init(Object *obj)
> for (i = 0;;) {
> struct ipmi_sdr_header *sdrh;
> int len;
> -if ((i + 5) > sizeof(init_sdrs)) {
> +if ((i + IPMI_SDR_HEADER_SIZE) > sizeof(init_sdrs)) {
> error_report("Problem with recid 0x%4.4x", i);
> return;
> }
> sdrh = (struct ipmi_sdr_header *) &init_sdrs[i];
> -len = sdrh->rec_length;
> +len = ipmi_sdr_length(sdrh);
> recid = ipmi_sdr_recid(sdrh);
> if (recid == 0x) {
> break;
> }
> -if ((i + len + 5) > sizeof(init_sdrs)) {
> +if ((i + len) > sizeof(init_sdrs)) {
> error_report("Problem with recid 0x%4.4x", i);
> return;
> }
> sdr_add_entry(ibs, init_sdrs + i, len, NULL);
> -i += len + 5;
> +i += len;
> }
>
> ipmi_init_sensors_from_sdrs(ibs);
> diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
> index 7e142e241dcb..74a2b5af9613 100644
> --- a/include/hw/ipmi/ipmi.h
> +++ b/include/hw/ipmi/ipmi.h
> @@ -219,6 +219,7 @@ struct ipmi_sdr_header {
> #define IPMI_SDR_HEADER_SIZE sizeof(struct ipmi_sdr_header)
>
> #define ipmi_sdr_recid(sdr) ((sdr)->rec_id[0] | ((sdr)->rec_id[1] << 8))
> +#define ipmi_sdr_length(sdr) ((sdr)->rec_length + IPMI_SDR_HEADER_SIZE)
>
> /*
> * 43.2 SDR Type 02h. Compact Sensor Record
[Qemu-devel] [PATCH] seabios: fix submodule
Commit "36f96c4 target-i386: Add support to migrate vcpu's TSC rate" updates roms/seabios, appearently by mistake. Revert this. Signed-off-by: Gerd Hoffmann --- roms/seabios | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roms/seabios b/roms/seabios index 33fbe13..01a84be 16 --- a/roms/seabios +++ b/roms/seabios @@ -1 +1 @@ -Subproject commit 33fbe13a3e2a01e0ba1087a8feed801a0451db21 +Subproject commit 01a84bea2d28a19d2405c1ecac4bdef17683cc0c -- 1.8.3.1
Re: [Qemu-devel] [PATCH] linux-user: add option to intercept execve() syscalls
Le 22/01/2016 11:47, Peter Maydell a écrit : > On 22 January 2016 at 10:33, Laurent Vivier wrote: >> Le 22/01/2016 11:01, Petros Angelatos a écrit : >>> This was my initial approach too, but argv[0] can be just the filename >>> like "qemu-arm-static". And while I could add extra logic to look this >>> up in the PATH, someone could run it from a completely different >>> location. Then I looked for a way to get the path of the current >>> executable but every platform has its own way of doing that and I >>> didn't want to add all these cases. >>> >>> https://stackoverflow.com/questions/1023306/finding-current-executables-path-without-proc-self-exe >> >> linux-user works only on linux. >> qemu uses glib-2.0, so you can use g_find_program_in_path(). > > If QEMU was started via execle() to set the environment of the > executed process and that specified environment has a different > PATH, then g_find_program_in_path() will give the wrong answer. > Using AT_EXECFN (perhaps with a fallback to /proc/self/exe) seems > like a better approach to me. I agree, you can use getauxval(AT_EXECFN). >>> questions. Is it ok that I deleted part of the patch for my reply to >>> code review, or should I have replied inline without deleting >> >> Generally, it's better to not delete parts. So, someone tacking the mail >> thread at a moment can read the whole history in the last mail. > > I tend to happily delete parts and assume that readers have > access to the thread (via the archive or in their mail readers). > Not deleting bits makes it hard to read replies if there's > a conversation about a small part of a large patch. Yes, I do that also... :) Laurent
Re: [Qemu-devel] [PATCH v1] kvm/x86: Hyper-V tsc page setup
On 22/01/2016 11:15, Andrey Smetanin wrote: >> >> - unless KVM can use a master clock, it is incorrect to set up the TSC >> page this way; the sequence needs to be 0x in that case > 0x is not an invalid value for tsc page, > see https://lkml.org/lkml/2015/11/2/655 oh, I see now. >> - writing the TSC page must be done while all VCPUs are stopped, because >> the TSC page doesn't provide the possibility for the guest to retry in >> the middle of an update (like seqcount in Linux doess) > I think Windows guest gives tsc page address at boot time and protects > against other vcpu's tsc page access. Sometimes the TSC is detected to be unstable and Linux switches to another clocksource. At least in that case you can get a write to the TSC page while the guest is running. In that case it would be enough to write a zero to tsc_sequence, which _can_ be done atomically while the guest is running. However, KVM already has a mechanism to stop all VCPUs (KVM_REQ_MASTERCLOCK_UPDATE) so we might as well use it. Paolo
Re: [Qemu-devel] [PATCH v2 6/9] ipmi: add get and set SENSOR_TYPE commands
On Thu, 21 Jan 2016 18:18:51 +0100
Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater
> Acked-by: Corey Minyard
> ---
Reviewed-by: Greg Kurz
Just two nits below.
> hw/ipmi/ipmi_bmc_sim.c | 45 -
> 1 file changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 803c7e5130c0..7c0f2a1d9799 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -42,6 +42,8 @@
> #define IPMI_CMD_REARM_SENSOR_EVTS0x2a
> #define IPMI_CMD_GET_SENSOR_EVT_STATUS0x2b
> #define IPMI_CMD_GET_SENSOR_READING 0x2d
> +#define IPMI_CMD_SET_SENSOR_TYPE 0x2e
> +#define IPMI_CMD_GET_SENSOR_TYPE 0x2f
>
> /* #define IPMI_NETFN_APP 0x06 In ipmi.h */
>
> @@ -1527,6 +1529,45 @@ static void get_sensor_reading(IPMIBmcSim *ibs,
> }
> }
>
> +static void set_sensor_type(IPMIBmcSim *ibs,
> + uint8_t *cmd, unsigned int cmd_len,
> + uint8_t *rsp, unsigned int *rsp_len,
> + unsigned int max_rsp_len)
> +{
> +IPMISensor *sens;
> +
> +
> +IPMI_CHECK_CMD_LEN(5);
> +if ((cmd[2] > MAX_SENSORS) ||
This has been a recurring remark on many patches lately, and all the people
don't necessarily agree but the extra parenthesis are not needed here...
> +!IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> +rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +return;
> +}
> +sens = ibs->sensors + cmd[2];
> +sens->sensor_type = cmd[3];
> +sens->evt_reading_type_code = cmd[4] & 0x7f;
> +}
> +
> +static void get_sensor_type(IPMIBmcSim *ibs,
> + uint8_t *cmd, unsigned int cmd_len,
> + uint8_t *rsp, unsigned int *rsp_len,
> + unsigned int max_rsp_len)
> +{
> +IPMISensor *sens;
> +
> +
> +IPMI_CHECK_CMD_LEN(3);
> +if ((cmd[2] > MAX_SENSORS) ||
and here.
> +!IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> +rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +return;
> +}
> +sens = ibs->sensors + cmd[2];
> +IPMI_ADD_RSP_DATA(sens->sensor_type);
> +IPMI_ADD_RSP_DATA(sens->evt_reading_type_code);
> +}
> +
> +
> static const IPMICmdHandler chassis_cmds[] = {
> [IPMI_CMD_GET_CHASSIS_CAPABILITIES] = chassis_capabilities,
> [IPMI_CMD_GET_CHASSIS_STATUS] = chassis_status,
> @@ -1542,7 +1583,9 @@ static const IPMICmdHandler sensor_event_cmds[] = {
> [IPMI_CMD_GET_SENSOR_EVT_ENABLE] = get_sensor_evt_enable,
> [IPMI_CMD_REARM_SENSOR_EVTS] = rearm_sensor_evts,
> [IPMI_CMD_GET_SENSOR_EVT_STATUS] = get_sensor_evt_status,
> -[IPMI_CMD_GET_SENSOR_READING] = get_sensor_reading
> +[IPMI_CMD_GET_SENSOR_READING] = get_sensor_reading,
> +[IPMI_CMD_SET_SENSOR_TYPE] = set_sensor_type,
> +[IPMI_CMD_GET_SENSOR_TYPE] = get_sensor_type,
> };
> static const IPMINetfn sensor_event_netfn = {
> .cmd_nums = ARRAY_SIZE(sensor_event_cmds),
Re: [Qemu-devel] [PATCH v2 7/9] ipmi: add GET_SYS_RESTART_CAUSE chassis command
On Thu, 21 Jan 2016 18:18:52 +0100
Cédric Le Goater wrote:
> This is a simulator. Just return an unknown cause (0).
>
> Signed-off-by: Cédric Le Goater
> Acked-by: Corey Minyard
> ---
Reviewed-by: Greg Kurz
> hw/ipmi/ipmi_bmc_sim.c | 16 +++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 7c0f2a1d9799..e882af3f1b40 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -34,6 +34,7 @@
> #define IPMI_CMD_GET_CHASSIS_CAPABILITIES 0x00
> #define IPMI_CMD_GET_CHASSIS_STATUS 0x01
> #define IPMI_CMD_CHASSIS_CONTROL 0x02
> +#define IPMI_CMD_GET_SYS_RESTART_CAUSE0x09
>
> #define IPMI_NETFN_SENSOR_EVENT 0x04
>
> @@ -197,6 +198,8 @@ struct IPMIBmcSim {
> uint8_t mfg_id[3];
> uint8_t product_id[2];
>
> +uint8_t restart_cause;
> +
> IPMISel sel;
> IPMISdr sdr;
> IPMISensor sensors[MAX_SENSORS];
> @@ -756,6 +759,15 @@ static void chassis_control(IPMIBmcSim *ibs,
> }
> }
>
> +static void chassis_get_sys_restart_cause(IPMIBmcSim *ibs,
> + uint8_t *cmd, unsigned int cmd_len,
> + uint8_t *rsp, unsigned int *rsp_len,
> + unsigned int max_rsp_len)
> +{
> +IPMI_ADD_RSP_DATA(ibs->restart_cause & 0xf); /* Restart Cause */
> +IPMI_ADD_RSP_DATA(0); /* Channel 0 */
> +}
> +
> static void get_device_id(IPMIBmcSim *ibs,
>uint8_t *cmd, unsigned int cmd_len,
>uint8_t *rsp, unsigned int *rsp_len,
> @@ -1571,7 +1583,8 @@ static void get_sensor_type(IPMIBmcSim *ibs,
> static const IPMICmdHandler chassis_cmds[] = {
> [IPMI_CMD_GET_CHASSIS_CAPABILITIES] = chassis_capabilities,
> [IPMI_CMD_GET_CHASSIS_STATUS] = chassis_status,
> -[IPMI_CMD_CHASSIS_CONTROL] = chassis_control
> +[IPMI_CMD_CHASSIS_CONTROL] = chassis_control,
> +[IPMI_CMD_GET_SYS_RESTART_CAUSE] = chassis_get_sys_restart_cause
> };
> static const IPMINetfn chassis_netfn = {
> .cmd_nums = ARRAY_SIZE(chassis_cmds),
> @@ -1692,6 +1705,7 @@ static void ipmi_sim_init(Object *obj)
> ibs->bmc_global_enables = (1 << IPMI_BMC_EVENT_LOG_BIT);
> ibs->device_id = 0x20;
> ibs->ipmi_version = 0x02; /* IPMI 2.0 */
> +ibs->restart_cause = 0;
> for (i = 0; i < 4; i++) {
> ibs->sel.last_addition[i] = 0xff;
> ibs->sel.last_clear[i] = 0xff;
Re: [Qemu-devel] [PATCH v2 4/9] ipmi: introduce a struct ipmi_sdr_compact
On 01/22/2016 11:49 AM, Greg Kurz wrote:
> On Thu, 21 Jan 2016 18:18:49 +0100
> Cédric Le Goater wrote:
>
>> Currently, sdr attributes are identified using byte offsets and this
>> can be a bit confusing.
>>
>> This patch adds a struct ipmi_sdr_compact conforming to the IPMI specs
>> and replaces byte offsets with names. It also introduces and uses a
>> struct ipmi_sdr_header in sections of the code where no assumption is
>> made on the type of SDR. This leave rooms to potential usage of other
>> types in the future.
>>
>
> Turning all these magic numbers into understandable names is definitely a
> great idea !
>
> See comments below.
>
>> Signed-off-by: Cédric Le Goater
>> ---
>> hw/ipmi/ipmi_bmc_sim.c | 65
>> +++---
>> include/hw/ipmi/ipmi.h | 44 ++
>> 2 files changed, 84 insertions(+), 25 deletions(-)
>>
>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
>> index fc596a548df7..31f990199154 100644
>> --- a/hw/ipmi/ipmi_bmc_sim.c
>> +++ b/hw/ipmi/ipmi_bmc_sim.c
>> @@ -323,11 +323,15 @@ static void sdr_inc_reservation(IPMISdr *sdr)
>> static int sdr_add_entry(IPMIBmcSim *ibs, const uint8_t *entry,
>> unsigned int len, uint16_t *recid)
>> {
>> +struct ipmi_sdr_header *sdrh_entry = (struct ipmi_sdr_header *) entry;
>
> This looks like the entry argument should be struct ipmi_sdr_header * and
> you would not need sdrh_entry.
Indeed and it improves readability a little more. I will send a fix in next
patchset.
>> +struct ipmi_sdr_header *sdrh =
>> +(struct ipmi_sdr_header *) &ibs->sdr.sdr[ibs->sdr.next_free];
>> +
>> if ((len < 5) || (len > 255)) {
>> return 1;
>> }
>>
>> -if (entry[4] != len - 5) {
>> +if (sdrh_entry->rec_length != len - 5) {
>> return 1;
>> }
>>
>> @@ -336,10 +340,10 @@ static int sdr_add_entry(IPMIBmcSim *ibs, const
>> uint8_t *entry,
>> return 1;
>> }
>>
>> -memcpy(ibs->sdr.sdr + ibs->sdr.next_free, entry, len);
>> -ibs->sdr.sdr[ibs->sdr.next_free] = ibs->sdr.next_rec_id & 0xff;
>> -ibs->sdr.sdr[ibs->sdr.next_free+1] = (ibs->sdr.next_rec_id >> 8) & 0xff;
>> -ibs->sdr.sdr[ibs->sdr.next_free+2] = 0x51; /* Conform to IPMI 1.5 spec
>> */
>> +memcpy(sdrh, entry, len);
>> +sdrh->rec_id[0] = ibs->sdr.next_rec_id & 0xff;
>> +sdrh->rec_id[1] = (ibs->sdr.next_rec_id >> 8) & 0xff;
>> +sdrh->sdr_version = 0x51; /* Conform to IPMI 1.5 spec */
>>
>> if (recid) {
>> *recid = ibs->sdr.next_rec_id;
>> @@ -357,8 +361,10 @@ static int sdr_find_entry(IPMISdr *sdr, uint16_t recid,
>> unsigned int pos = *retpos;
>>
>> while (pos < sdr->next_free) {
>> -uint16_t trec = sdr->sdr[pos] | (sdr->sdr[pos + 1] << 8);
>> -unsigned int nextpos = pos + sdr->sdr[pos + 4];
>> +struct ipmi_sdr_header *sdrh =
>> +(struct ipmi_sdr_header *) &sdr->sdr[pos];
>> +uint16_t trec = ipmi_sdr_recid(sdrh);
>> +unsigned int nextpos = pos + sdrh->rec_length;
>>
>> if (trec == recid) {
>> if (nextrec) {
>> @@ -507,29 +513,32 @@ static void ipmi_init_sensors_from_sdrs(IPMIBmcSim *s)
>>
>> pos = 0;
>> for (i = 0; !sdr_find_entry(&s->sdr, i, &pos, NULL); i++) {
>> -uint8_t *sdr = s->sdr.sdr + pos;
>> -unsigned int len = sdr[4];
>> +struct ipmi_sdr_compact *sdr =
>> +(struct ipmi_sdr_compact *) &s->sdr.sdr[pos];
>> +unsigned int len = sdr->header.rec_length;
>>
>> if (len < 20) {
>> continue;
>> }
>> -if ((sdr[3] < 1) || (sdr[3] > 2)) {
>> +if (sdr->header.rec_type != IPMI_SDR_COMPACT_TYPE) {
>> continue; /* Not a sensor SDR we set from */
>> }
>>
>> -if (sdr[7] > MAX_SENSORS) {
>> +if (sdr->sensor_owner_number > MAX_SENSORS) {
>> continue;
>> }
>> -sens = s->sensors + sdr[7];
>> +sens = s->sensors + sdr->sensor_owner_number;
>>
>> IPMI_SENSOR_SET_PRESENT(sens, 1);
>> -IPMI_SENSOR_SET_SCAN_ON(sens, (sdr[10] >> 6) & 1);
>> -IPMI_SENSOR_SET_EVENTS_ON(sens, (sdr[10] >> 5) & 1);
>> -sens->assert_suppt = sdr[14] | (sdr[15] << 8);
>> -sens->deassert_suppt = sdr[16] | (sdr[17] << 8);
>> -sens->states_suppt = sdr[18] | (sdr[19] << 8);
>> -sens->sensor_type = sdr[12];
>> -sens->evt_reading_type_code = sdr[13] & 0x7f;
>> +IPMI_SENSOR_SET_SCAN_ON(sens, (sdr->sensor_init >> 6) & 1);
>> +IPMI_SENSOR_SET_EVENTS_ON(sens, (sdr->sensor_init >> 5) & 1);
>> +sens->assert_suppt = sdr->assert_mask[0] | (sdr->assert_mask[1] <<
>> 8);
>> +sens->deassert_suppt =
>> +sdr->deassert_mask[0] | (sdr->deassert_mask[1] << 8);
>> +sens->states_suppt =
>> +sdr->discrete_mask[0] | (sdr->discrete_mask[1] << 8);
>> +sens->sensor_type = sdr->
Re: [Qemu-devel] [PATCH v1] kvm/x86: Hyper-V tsc page setup
On 01/22/2016 02:02 PM, Paolo Bonzini wrote: On 22/01/2016 11:15, Andrey Smetanin wrote: - unless KVM can use a master clock, it is incorrect to set up the TSC page this way; the sequence needs to be 0x in that case 0x is not an invalid value for tsc page, see https://lkml.org/lkml/2015/11/2/655 oh, I see now. - writing the TSC page must be done while all VCPUs are stopped, because the TSC page doesn't provide the possibility for the guest to retry in the middle of an update (like seqcount in Linux doess) I think Windows guest gives tsc page address at boot time and protects against other vcpu's tsc page access. Sometimes the TSC is detected to be unstable and Linux switches to another clocksource. At least in that case you can get a write to the TSC page while the guest is running. I can't understand how write is possible. Linux Hyper-V driver hv_vmbus.ko does the following inside hv_init() drivers/hv/hv.c(line 256): wrmsrl(HV_X64_MSR_REFERENCE_TSC, tsc_msr.as_uint64); clocksource_register_hz(&hyperv_cs_tsc, NSEC_PER_SEC/100); So page is setup only once before registration clock source. In that case it would be enough to write a zero to tsc_sequence, which _can_ be done atomically while the guest is running. However, KVM already has a mechanism to stop all VCPUs (KVM_REQ_MASTERCLOCK_UPDATE) so we might as well use it. Paolo
Re: [Qemu-devel] [PATCH v2 6/9] ipmi: add get and set SENSOR_TYPE commands
On 01/22/2016 12:07 PM, Greg Kurz wrote:
> On Thu, 21 Jan 2016 18:18:51 +0100
> Cédric Le Goater wrote:
>
>> Signed-off-by: Cédric Le Goater
>> Acked-by: Corey Minyard
>> ---
>
> Reviewed-by: Greg Kurz
>
> Just two nits below.
>
>> hw/ipmi/ipmi_bmc_sim.c | 45 -
>> 1 file changed, 44 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
>> index 803c7e5130c0..7c0f2a1d9799 100644
>> --- a/hw/ipmi/ipmi_bmc_sim.c
>> +++ b/hw/ipmi/ipmi_bmc_sim.c
>> @@ -42,6 +42,8 @@
>> #define IPMI_CMD_REARM_SENSOR_EVTS0x2a
>> #define IPMI_CMD_GET_SENSOR_EVT_STATUS0x2b
>> #define IPMI_CMD_GET_SENSOR_READING 0x2d
>> +#define IPMI_CMD_SET_SENSOR_TYPE 0x2e
>> +#define IPMI_CMD_GET_SENSOR_TYPE 0x2f
>>
>> /* #define IPMI_NETFN_APP 0x06 In ipmi.h */
>>
>> @@ -1527,6 +1529,45 @@ static void get_sensor_reading(IPMIBmcSim *ibs,
>> }
>> }
>>
>> +static void set_sensor_type(IPMIBmcSim *ibs,
>> + uint8_t *cmd, unsigned int cmd_len,
>> + uint8_t *rsp, unsigned int *rsp_len,
>> + unsigned int max_rsp_len)
>> +{
>> +IPMISensor *sens;
>> +
>> +
>> +IPMI_CHECK_CMD_LEN(5);
>> +if ((cmd[2] > MAX_SENSORS) ||
>
> This has been a recurring remark on many patches lately, and all the people
> don't necessarily agree but the extra parenthesis are not needed here...
Damn. Am I contaminated ? :)
C.
>> +!IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
>> +rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +return;
>> +}
>> +sens = ibs->sensors + cmd[2];
>> +sens->sensor_type = cmd[3];
>> +sens->evt_reading_type_code = cmd[4] & 0x7f;
>> +}
>> +
>> +static void get_sensor_type(IPMIBmcSim *ibs,
>> + uint8_t *cmd, unsigned int cmd_len,
>> + uint8_t *rsp, unsigned int *rsp_len,
>> + unsigned int max_rsp_len)
>> +{
>> +IPMISensor *sens;
>> +
>> +
>> +IPMI_CHECK_CMD_LEN(3);
>> +if ((cmd[2] > MAX_SENSORS) ||
>
> and here.
>
>> +!IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
>> +rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
>> +return;
>> +}
>> +sens = ibs->sensors + cmd[2];
>> +IPMI_ADD_RSP_DATA(sens->sensor_type);
>> +IPMI_ADD_RSP_DATA(sens->evt_reading_type_code);
>> +}
>> +
>> +
>> static const IPMICmdHandler chassis_cmds[] = {
>> [IPMI_CMD_GET_CHASSIS_CAPABILITIES] = chassis_capabilities,
>> [IPMI_CMD_GET_CHASSIS_STATUS] = chassis_status,
>> @@ -1542,7 +1583,9 @@ static const IPMICmdHandler sensor_event_cmds[] = {
>> [IPMI_CMD_GET_SENSOR_EVT_ENABLE] = get_sensor_evt_enable,
>> [IPMI_CMD_REARM_SENSOR_EVTS] = rearm_sensor_evts,
>> [IPMI_CMD_GET_SENSOR_EVT_STATUS] = get_sensor_evt_status,
>> -[IPMI_CMD_GET_SENSOR_READING] = get_sensor_reading
>> +[IPMI_CMD_GET_SENSOR_READING] = get_sensor_reading,
>> +[IPMI_CMD_SET_SENSOR_TYPE] = set_sensor_type,
>> +[IPMI_CMD_GET_SENSOR_TYPE] = get_sensor_type,
>> };
>> static const IPMINetfn sensor_event_netfn = {
>> .cmd_nums = ARRAY_SIZE(sensor_event_cmds),
>
Re: [Qemu-devel] [PATCH v2 5/9] ipmi: fix SDR length value
On 01/22/2016 11:56 AM, Greg Kurz wrote:
> On Thu, 21 Jan 2016 18:18:50 +0100
> Cédric Le Goater wrote:
>> The IPMI BMC simulator populates the SDR table with a set of initial
>> SDRs. The length of each SDR is taken from the record itself (byte 4)
>> which does not include the size of the header. But, the full length
>> (header + data) is required by the sdr_add_entry() routine.
>>
>> Signed-off-by: Cédric Le Goater
>
> The patch is good but IMHO it should come before patch 4 because this is
> bugfix
> that could be applied right away, while patch 4 is code cleanup that may need
> some more discussion.
OK. I am fine with that. It should be the patch from v1.
Thanks,
C.
>> ---
>> hw/ipmi/ipmi_bmc_sim.c | 18 +-
>> include/hw/ipmi/ipmi.h | 1 +
>> 2 files changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
>> index 31f990199154..803c7e5130c0 100644
>> --- a/hw/ipmi/ipmi_bmc_sim.c
>> +++ b/hw/ipmi/ipmi_bmc_sim.c
>> @@ -327,11 +327,11 @@ static int sdr_add_entry(IPMIBmcSim *ibs, const
>> uint8_t *entry,
>> struct ipmi_sdr_header *sdrh =
>> (struct ipmi_sdr_header *) &ibs->sdr.sdr[ibs->sdr.next_free];
>>
>> -if ((len < 5) || (len > 255)) {
>> +if ((len < IPMI_SDR_HEADER_SIZE) || (len > 255)) {
>> return 1;
>> }
>>
>> -if (sdrh_entry->rec_length != len - 5) {
>> +if (ipmi_sdr_length(sdrh_entry) != len) {
>> return 1;
>> }
>>
>> @@ -364,7 +364,7 @@ static int sdr_find_entry(IPMISdr *sdr, uint16_t recid,
>> struct ipmi_sdr_header *sdrh =
>> (struct ipmi_sdr_header *) &sdr->sdr[pos];
>> uint16_t trec = ipmi_sdr_recid(sdrh);
>> -unsigned int nextpos = pos + sdrh->rec_length;
>> +unsigned int nextpos = pos + ipmi_sdr_length(sdrh);
>>
>> if (trec == recid) {
>> if (nextrec) {
>> @@ -1179,7 +1179,7 @@ static void get_sdr(IPMIBmcSim *ibs,
>>
>> sdrh = (struct ipmi_sdr_header *) &ibs->sdr.sdr[pos];
>>
>> -if (cmd[6] > sdrh->rec_length) {
>> +if (cmd[6] > ipmi_sdr_length(sdrh)) {
>> rsp[2] = IPMI_CC_PARM_OUT_OF_RANGE;
>> return;
>> }
>> @@ -1188,7 +1188,7 @@ static void get_sdr(IPMIBmcSim *ibs,
>> IPMI_ADD_RSP_DATA((nextrec >> 8) & 0xff);
>>
>> if (cmd[7] == 0xff) {
>> -cmd[7] = sdrh->rec_length - cmd[6];
>> +cmd[7] = ipmi_sdr_length(sdrh) - cmd[6];
>> }
>>
>> if ((cmd[7] + *rsp_len) > max_rsp_len) {
>> @@ -1659,22 +1659,22 @@ static void ipmi_sim_init(Object *obj)
>> for (i = 0;;) {
>> struct ipmi_sdr_header *sdrh;
>> int len;
>> -if ((i + 5) > sizeof(init_sdrs)) {
>> +if ((i + IPMI_SDR_HEADER_SIZE) > sizeof(init_sdrs)) {
>> error_report("Problem with recid 0x%4.4x", i);
>> return;
>> }
>> sdrh = (struct ipmi_sdr_header *) &init_sdrs[i];
>> -len = sdrh->rec_length;
>> +len = ipmi_sdr_length(sdrh);
>> recid = ipmi_sdr_recid(sdrh);
>> if (recid == 0x) {
>> break;
>> }
>> -if ((i + len + 5) > sizeof(init_sdrs)) {
>> +if ((i + len) > sizeof(init_sdrs)) {
>> error_report("Problem with recid 0x%4.4x", i);
>> return;
>> }
>> sdr_add_entry(ibs, init_sdrs + i, len, NULL);
>> -i += len + 5;
>> +i += len;
>> }
>>
>> ipmi_init_sensors_from_sdrs(ibs);
>> diff --git a/include/hw/ipmi/ipmi.h b/include/hw/ipmi/ipmi.h
>> index 7e142e241dcb..74a2b5af9613 100644
>> --- a/include/hw/ipmi/ipmi.h
>> +++ b/include/hw/ipmi/ipmi.h
>> @@ -219,6 +219,7 @@ struct ipmi_sdr_header {
>> #define IPMI_SDR_HEADER_SIZE sizeof(struct ipmi_sdr_header)
>>
>> #define ipmi_sdr_recid(sdr) ((sdr)->rec_id[0] | ((sdr)->rec_id[1] << 8))
>> +#define ipmi_sdr_length(sdr) ((sdr)->rec_length + IPMI_SDR_HEADER_SIZE)
>>
>> /*
>> * 43.2 SDR Type 02h. Compact Sensor Record
>
Re: [Qemu-devel] [PATCH v2 3/3] target-arm: Implement the S2 MMU inputsize > pamax check
On Fri, Jan 22, 2016 at 10:28:43AM +, Alex Bennée wrote:
>
> Edgar E. Iglesias writes:
>
> > From: "Edgar E. Iglesias"
> >
> > Implement the inputsize > pamax check for Stage 2 translations.
> > We have multiple choices for how to respond to errors and
> > choose to fault.
> >
> > Signed-off-by: Edgar E. Iglesias
> > ---
> > target-arm/helper.c | 16
> > 1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> > index 4abeb4d..9a7ff5e 100644
> > --- a/target-arm/helper.c
> > +++ b/target-arm/helper.c
> > @@ -6808,7 +6808,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> > target_ulong address,
> > */
> > int startlevel = extract32(tcr->raw_tcr, 6, 2);
> > unsigned int pamax = arm_pamax(cpu);
> > -bool ok;
> > +bool ok = true;
> >
> > if (va_size == 32 || stride == 9) {
> > /* AArch32 or 4KB pages */
> > @@ -6818,9 +6818,17 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> > target_ulong address,
> > level = 3 - startlevel;
> > }
> >
> > -/* Check that the starting level is valid. */
> > -ok = check_s2_startlevel(cpu, va_size == 64, level,
> > - inputsize, stride, pamax);
> > +if (va_size == 64 &&
> > +inputsize > pamax &&
> > +(arm_el_is_aa64(env, 1) || inputsize > 40)) {
>
> If va_size == 64 doesn't that imply arm_el_is_aa64(env, 1)? Looking
> further up the function it seems that is what sets va_size in the first
> place. I think that makes the inputsize > 40 check redundant.
va_size == 64 is true if the EL corresponding to the translation _regime_
is in 64 bit mode (in this case EL2).
EL1 may very well be in 32bit mode.
>
> > +/* We have multiple choices but choose to fault. */
> > +ok = false;
> > +}
> > +if (ok) {
> > +/* Check that the starting level is valid. */
> > +ok = check_s2_startlevel(cpu, va_size == 64, level,
> > + inputsize, stride, pamax);
> > +}
> > if (!ok) {
> > /* AArch64 reports these as level 0 faults.
> > * AArch32 reports these as level 1 faults.
>
> I'm not a fan of the ok = true / ok = false / ok =
> check_s2_start_level() / if (!ok) ping-pong here as it is hard to
> follow. I'm not sure how you could make it cleaner to follow though.
> Maybe something like:
>
> /* For stage 2 translations the starting level is specified by the
> * VTCR_EL2.SL0 field (whose interpretation depends on the page size)
> */
> int startlevel = extract32(tcr->raw_tcr, 6, 2);
> unsigned int pamax = arm_pamax(cpu);
> bool is_aarch64_regime = (va_size == 64);
> bool ok;
>
> if (va_size == 32 || stride == 9) {
> /* AArch32 or 4KB pages */
> level = 2 - startlevel;
> } else {
> /* 16KB or 64KB pages */
> level = 3 - startlevel;
> }
>
> if (is_aarch64_regime &&
> inputsize > pamax) {
> /* We have multiple choices but choose to fault. */
> ok = false;
> } else {
> /* Check that the starting level is valid. */
> ok = check_s2_startlevel(cpu, is_aarch64_regime, level,
> inputsize, stride, pamax);
> }
> if (!ok) {
> /* AArch64 reports these as level 0 faults.
> * AArch32 reports these as level 1 faults.
> */
> level = is_aarch64_regime ? 0 : 1;
> fault_type = translation_fault;
> goto do_fault;
> }
>
> But I'm wondering if it just makes more sense to push the:
>
> is_aarch64_regime && inputsize > pamax
>
> Check into check_s2_startlevel? Then you could just have a simple call
> which succeeds or falls through to a fault?
Yeah, I guess we could rename check_s2_startlevel to something more generic
and move all the checks there. I don't feel very strongly about either way...
Thanks,
Edgar
>
> /* Check that the starting level is valid. */
> if (!check_s2_startlevel(cpu, is_aarch64_regime, level,
> inputsize, stride, pamax) ){
> /* AArch64 reports these as level 0 faults.
> * AArch32 reports these as level 1 faults.
> */
> level = is_aarch64_regime ? 0 : 1;
> fault_type = translation_fault;
> goto do_fault;
> }
>
> --
> Alex Bennée
Re: [Qemu-devel] [PULL 10/11] Add Error **errp for xen_pt_config_init()
On 21/01/2016 18:01, Stefano Stabellini wrote:
> -XEN_PT_LOG(&s->dev, "Failed to initialize %d/%ld reg
> 0x%x in grp_type=0x%x (%d/%ld), rc=%d\n",
> - j,
> ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
> - regs->offset,
> xen_pt_emu_reg_grps[i].grp_type,
> - i, ARRAY_SIZE(xen_pt_emu_reg_grps), rc);
> +xen_pt_config_reg_init(s, reg_grp_entry, regs, &err);
> +if (err) {
> +error_append_hint(&err, "Failed to initialize %d/%zu"
> +" reg 0x%x in grp_type = 0x%x (%d/%zu)",
> +j,
> ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
Coverity noticed a preexisting problem here. emu_regs is a pointer,
thus ARRAY_SIZE doesn't return what you expect.
Paolo
> +regs->offset,
> xen_pt_emu_reg_grps[i].grp_type,
> +i, ARRAY_SIZE(xen_pt_emu_reg_grps));
> +error_propagate(errp, err);
Re: [Qemu-devel] [PULL 12/17] qcow2: Implement .bdrv_inactivate
On 20/01/2016 17:25, Kevin Wolf wrote:
> +ret = qcow2_cache_flush(bs, s->refcount_block_cache);
> +if (ret) {
> +result = ret;
> +error_report("Failed to flush the refcount block cache: %s",
> + strerror(-ret));
> +}
> +
> +if (result == 0) {
> +qcow2_mark_clean(bs);
Should you return an error if qcow2_mark_clean does? (Coverity noticed
that its return value is usually checked).
Paolo
> +}
> +
> +return result;
Re: [Qemu-devel] [PATCH v2 8/9] ipmi: add ACPI power and GUID commands
On Thu, 21 Jan 2016 18:18:53 +0100
Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater
> ---
>
> Changes since v1:
> - added ACPI to command names.
>
> hw/ipmi/ipmi_bmc_sim.c | 49 +
> 1 file changed, 49 insertions(+)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index e882af3f1b40..53c75cb21c1a 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -25,6 +25,7 @@
> #include
> #include
> #include
> +#include "sysemu/sysemu.h"
> #include "qemu/timer.h"
> #include "hw/ipmi/ipmi.h"
> #include "qemu/error-report.h"
> @@ -51,6 +52,9 @@
> #define IPMI_CMD_GET_DEVICE_ID0x01
> #define IPMI_CMD_COLD_RESET 0x02
> #define IPMI_CMD_WARM_RESET 0x03
> +#define IPMI_CMD_SET_ACPI_POWER_STATE 0x06
> +#define IPMI_CMD_GET_ACPI_POWER_STATE 0x07
> +#define IPMI_CMD_GET_DEVICE_GUID 0x08
> #define IPMI_CMD_RESET_WATCHDOG_TIMER 0x22
> #define IPMI_CMD_SET_WATCHDOG_TIMER 0x24
> #define IPMI_CMD_GET_WATCHDOG_TIMER 0x25
> @@ -200,6 +204,9 @@ struct IPMIBmcSim {
>
> uint8_t restart_cause;
>
> +uint8_t acpi_power_state[2];
> +uint8_t uuid[16];
> +
> IPMISel sel;
> IPMISdr sdr;
> IPMISensor sensors[MAX_SENSORS];
> @@ -828,6 +835,36 @@ static void warm_reset(IPMIBmcSim *ibs,
> k->reset(s, false);
> }
> }
> +static void set_acpi_power_state(IPMIBmcSim *ibs,
> + uint8_t *cmd, unsigned int cmd_len,
> + uint8_t *rsp, unsigned int *rsp_len,
> + unsigned int max_rsp_len)
> +{
> +IPMI_CHECK_CMD_LEN(4);
> +ibs->acpi_power_state[0] = cmd[2];
> +ibs->acpi_power_state[1] = cmd[3];
> +}
> +
> +static void get_acpi_power_state(IPMIBmcSim *ibs,
> + uint8_t *cmd, unsigned int cmd_len,
> + uint8_t *rsp, unsigned int *rsp_len,
> + unsigned int max_rsp_len)
> +{
> +IPMI_ADD_RSP_DATA(ibs->acpi_power_state[0]);
> +IPMI_ADD_RSP_DATA(ibs->acpi_power_state[1]);
> +}
> +
> +static void get_device_guid(IPMIBmcSim *ibs,
> + uint8_t *cmd, unsigned int cmd_len,
> + uint8_t *rsp, unsigned int *rsp_len,
> + unsigned int max_rsp_len)
> +{
> +unsigned int i;
> +
> +for (i = 0; i < 16; i++) {
> +IPMI_ADD_RSP_DATA(ibs->uuid[i]);
> +}
> +}
>
> static void set_bmc_global_enables(IPMIBmcSim *ibs,
> uint8_t *cmd, unsigned int cmd_len,
> @@ -1609,6 +1646,9 @@ static const IPMICmdHandler app_cmds[] = {
> [IPMI_CMD_GET_DEVICE_ID] = get_device_id,
> [IPMI_CMD_COLD_RESET] = cold_reset,
> [IPMI_CMD_WARM_RESET] = warm_reset,
> +[IPMI_CMD_SET_ACPI_POWER_STATE] = set_acpi_power_state,
> +[IPMI_CMD_GET_ACPI_POWER_STATE] = get_acpi_power_state,
> +[IPMI_CMD_GET_DEVICE_GUID] = get_device_guid,
> [IPMI_CMD_SET_BMC_GLOBAL_ENABLES] = set_bmc_global_enables,
> [IPMI_CMD_GET_BMC_GLOBAL_ENABLES] = get_bmc_global_enables,
> [IPMI_CMD_CLR_MSG_FLAGS] = clr_msg_flags,
> @@ -1734,6 +1774,15 @@ static void ipmi_sim_init(Object *obj)
> i += len;
> }
>
> +ibs->acpi_power_state[0] = 0;
> +ibs->acpi_power_state[1] = 0;
> +
> +if (qemu_uuid_set) {
> +memcpy(&ibs->uuid, qemu_uuid, 16);
> +} else {
> +memset(&ibs->uuid, 0, 16);
> +}
> +
Sorry if this is a dumb question: why does the VM's UUID gets copied here ?
> ipmi_init_sensors_from_sdrs(ibs);
> register_cmds(ibs);
>
[Qemu-devel] [PATCH] qemu-sockets: simplify error handling
Just go always through the err label. (Noticed because Coverity
complains that peer is always non-NULL in the error cleanup code,
but removing the "if" is arguably more prone to introducing the
opposite bug in the future).
Signed-off-by: Paolo Bonzini
---
util/qemu-sockets.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index f455a17..a9fed00 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -464,7 +464,7 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
if (err) {
error_propagate(errp, err);
-return -1;
+goto err;
}
addr = sraddr->host;
@@ -474,13 +474,13 @@ static int inet_dgram_saddr(InetSocketAddress *sraddr,
}
if (port == NULL || strlen(port) == 0) {
error_setg(errp, "remote port not specified");
-return -1;
+goto err;
}
if (0 != (rc = getaddrinfo(addr, port, &ai, &peer))) {
error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
gai_strerror(rc));
- return -1;
+ goto err;
}
/* lookup local addr */
--
2.5.0
Re: [Qemu-devel] [PATCH v2 9/9] ipmi: add SET_SENSOR_READING command (tentative try)
On Thu, 21 Jan 2016 18:18:54 +0100
Cédric Le Goater wrote:
> SET_SENSOR_READING is a complex IPMI command (IPMI spec : "35.17 Set
> Sensor Reading And Event Status Command"). Here is a very minimum
> framework fitting the Open PowerNV platform needs. This command is
> used on this platform to set the "System Firmware Progress" sensor and
> the "Boot Count" sensor.
>
> Signed-off-by: Cédric Le Goater
> Acked-by: Corey Minyard
> ---
Reviewed-by: Greg Kurz
Just one "parenthesitis" attack...
> hw/ipmi/ipmi_bmc_sim.c | 135
> +
> 1 file changed, 135 insertions(+)
>
> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
> index 53c75cb21c1a..0aa7e67ae217 100644
> --- a/hw/ipmi/ipmi_bmc_sim.c
> +++ b/hw/ipmi/ipmi_bmc_sim.c
> @@ -46,6 +46,7 @@
> #define IPMI_CMD_GET_SENSOR_READING 0x2d
> #define IPMI_CMD_SET_SENSOR_TYPE 0x2e
> #define IPMI_CMD_GET_SENSOR_TYPE 0x2f
> +#define IPMI_CMD_SET_SENSOR_READING 0x30
>
> /* #define IPMI_NETFN_APP 0x06 In ipmi.h */
>
> @@ -1616,6 +1617,139 @@ static void get_sensor_type(IPMIBmcSim *ibs,
> IPMI_ADD_RSP_DATA(sens->evt_reading_type_code);
> }
>
> +static void set_sensor_reading(IPMIBmcSim *ibs,
> + uint8_t *cmd, unsigned int cmd_len,
> + uint8_t *rsp, unsigned int *rsp_len,
> + unsigned int max_rsp_len)
> +{
> +IPMISensor *sens;
> +uint8_t evd1;
> +uint8_t evd2;
> +uint8_t evd3;
> +
> +IPMI_CHECK_CMD_LEN(5);
> +if ((cmd[2] > MAX_SENSORS) ||
Here ! :)
> +!IPMI_SENSOR_GET_PRESENT(ibs->sensors + cmd[2])) {
> +rsp[2] = IPMI_CC_REQ_ENTRY_NOT_PRESENT;
> +return;
> +}
> +
> +sens = ibs->sensors + cmd[2];
> +
> +/* Sensor Reading operation */
> +switch ((cmd[3]) & 0x3) {
> +case 0: /* Do not change */
> +break;
> +case 1: /* write given value to sensor reading byte */
> +sens->reading = cmd[4];
> +break;
> +case 2:
> +case 3:
> +rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +return;
> +}
> +
> +/* Deassertion bits operation */
> +switch ((cmd[3] >> 2) & 0x3) {
> +case 0: /* Do not change */
> +break;
> +case 1: /* write given value */
> +if (cmd_len > 7) {
> +sens->deassert_states = cmd[7];
> +}
> +if (cmd_len > 8) {
> +sens->deassert_states = cmd[8] << 8;
> +}
> +
> +case 2: /* mask on */
> +if (cmd_len > 7) {
> +sens->deassert_states |= cmd[7];
> +}
> +if (cmd_len > 8) {
> +sens->deassert_states |= cmd[8] << 8;
> +}
> +break;
> +case 3: /* mask off */
> +if (cmd_len > 7) {
> +sens->deassert_states &= cmd[7];
> +}
> +if (cmd_len > 8) {
> +sens->deassert_states &= (cmd[8] << 8);
> +}
> +break;
> +}
> +
> +/* Assertion bits operation */
> +switch ((cmd[3] >> 4) & 0x3) {
> +case 0: /* Do not change */
> +break;
> +case 1: /* write given value */
> +if (cmd_len > 5) {
> +sens->assert_states = cmd[5];
> +}
> +if (cmd_len > 6) {
> +sens->assert_states = cmd[6] << 8;
> +}
> +
> +case 2: /* mask on */
> +if (cmd_len > 5) {
> +sens->assert_states |= cmd[5];
> +}
> +if (cmd_len > 6) {
> +sens->assert_states |= cmd[6] << 8;
> +}
> +break;
> +case 3: /* mask off */
> +if (cmd_len > 5) {
> +sens->assert_states &= cmd[5];
> +}
> +if (cmd_len > 6) {
> +sens->assert_states &= (cmd[6] << 8);
> +}
> +break;
> +}
> +
> +evd1 = evd2 = evd3 = 0x0;
> +if (cmd_len > 9) {
> +evd1 = cmd[9];
> +}
> +if (cmd_len > 10) {
> +evd2 = cmd[10];
> +}
> +if (cmd_len > 11) {
> +evd3 = cmd[11];
> +}
> +
> +/* Event Data Bytes operation */
> +switch ((cmd[3] >> 6) & 0x3) {
> +case 0: /* Do not use the event data in message */
> +evd1 = evd2 = evd3 = 0x0;
> +break;
> +case 1: /* Write given values to event data bytes excluding bits
> + * [3:0] Event Data 1. */
> +evd1 &= 0xf0;
> +break;
> +case 2: /* Write given values to event data bytes including bits
> + * [3:0] Event Data 1. */
> +break;
> +case 3:
> +rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
> +return;
> +}
> +
> +if (IPMI_SENSOR_IS_DISCRETE(sens)) {
> +unsigned int bit = evd1 & 0xf;
> +uint16_t mask = (1 << bit);
> +
> +if (sens->assert_states & mask & sens->assert_enable) {
> +gen_event(ibs, cmd[2], 0, evd1, evd2, evd3);
> +}
> +
> +if (sens->deassert_states & mask & sens->deassert_enable) {
> +
Re: [Qemu-devel] [PATCH] qemu-sockets: simplify error handling
On Fri, Jan 22, 2016 at 12:28:33PM +0100, Paolo Bonzini wrote: > Just go always through the err label. (Noticed because Coverity > complains that peer is always non-NULL in the error cleanup code, > but removing the "if" is arguably more prone to introducing the > opposite bug in the future). > > Signed-off-by: Paolo Bonzini Reviewed-by: Daniel P. Berrange Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v1] kvm/x86: Hyper-V tsc page setup
On 01/22/2016 02:02 PM, Paolo Bonzini wrote: On 22/01/2016 11:15, Andrey Smetanin wrote: - unless KVM can use a master clock, it is incorrect to set up the TSC page this way; the sequence needs to be 0x in that case 0x is not an invalid value for tsc page, see https://lkml.org/lkml/2015/11/2/655 oh, I see now. - writing the TSC page must be done while all VCPUs are stopped, because the TSC page doesn't provide the possibility for the guest to retry in the middle of an update (like seqcount in Linux doess) I think Windows guest gives tsc page address at boot time and protects against other vcpu's tsc page access. Sometimes the TSC is detected to be unstable and Linux switches to another clocksource. At least in that case you can get a write to the TSC page while the guest is running. Sorry, now I got it, you mean host TSC is unstable and we should mark guest tsc page invalid. Now I understand please ignore my prev. message. In that case it would be enough to write a zero to tsc_sequence, which _can_ be done atomically while the guest is running. However, KVM already has a mechanism to stop all VCPUs (KVM_REQ_MASTERCLOCK_UPDATE) so we might as well use it. Paolo
Re: [Qemu-devel] [PATCH v2 08/13] block: Support meta dirty bitmap
On 20.01.2016 09:11, Fam Zheng wrote:
The added group of operations enables tracking of the changed bits in
the dirty bitmap.
Signed-off-by: Fam Zheng
---
block/dirty-bitmap.c | 51
include/block/dirty-bitmap.h | 9
2 files changed, 60 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index bd7758b..d75dcf7 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -37,6 +37,7 @@
*/
struct BdrvDirtyBitmap {
HBitmap *bitmap;/* Dirty sector bitmap implementation */
+HBitmap *meta; /* Meta dirty bitmap */
BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
char *name; /* Optional non-empty unique ID */
int64_t size; /* Size of the bitmap (Number of sectors) */
@@ -102,6 +103,56 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState
*bs,
return bitmap;
}
+/* bdrv_create_meta_dirty_bitmap
+ *
+ * Create a meta dirty bitmap that tracks the changes of bits in @bitmap. I.e.
+ * when a dirty status bit in @bitmap is changed (either from reset to set or
+ * the other way around), its respective meta dirty bitmap bit will be marked
+ * dirty as well.
+ *
+ * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
+ * @granularity: how many bytes of bitmap data does each bit in the meta bitmap
+ * track.
+ */
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+ int granularity)
+{
+assert(!bitmap->meta);
+bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
+ BDRV_SECTOR_SIZE * BITS_PER_BYTE);
+}
what is granularity here? Is it unused?
Here should be chunk_size parameter, which then will be somehow send to
hbitmap_create_meta.
+
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+assert(bitmap->meta);
+hbitmap_free_meta(bitmap->bitmap);
+bitmap->meta = NULL;
+}
+
+int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors)
+{
+uint64_t i;
+int gran = bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
+
+/* To optimize: we can make hbitmap to internally check the range in a
+ * coarse level, or at least do it word by word. */
+for (i = sector; i < sector + nb_sectors; i += gran) {
+if (hbitmap_get(bitmap->meta, i)) {
+return true;
+}
+}
+return false;
+}
+
+void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors)
+{
+hbitmap_reset(bitmap->meta, sector, nb_sectors);
+}
+
bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
{
return bitmap->successor;
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 120bac6..d9b281a 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -9,6 +9,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
uint32_t granularity,
const char *name,
Error **errp);
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+ int granularity);
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap,
Error **errp);
@@ -35,6 +38,12 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
+int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors);
+void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors);
BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
uint64_t first_sector);
void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
Re: [Qemu-devel] [PATCH v2 3/3] target-arm: Implement the S2 MMU inputsize > pamax check
Edgar E. Iglesias writes:
> On Fri, Jan 22, 2016 at 10:28:43AM +, Alex Bennée wrote:
>>
>> Edgar E. Iglesias writes:
>>
>> > From: "Edgar E. Iglesias"
>> >
>> > Implement the inputsize > pamax check for Stage 2 translations.
>> > We have multiple choices for how to respond to errors and
>> > choose to fault.
>> >
>> > Signed-off-by: Edgar E. Iglesias
>> > ---
>> > target-arm/helper.c | 16
>> > 1 file changed, 12 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/target-arm/helper.c b/target-arm/helper.c
>> > index 4abeb4d..9a7ff5e 100644
>> > --- a/target-arm/helper.c
>> > +++ b/target-arm/helper.c
>> > @@ -6808,7 +6808,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
>> > target_ulong address,
>> > */
>> > int startlevel = extract32(tcr->raw_tcr, 6, 2);
>> > unsigned int pamax = arm_pamax(cpu);
>> > -bool ok;
>> > +bool ok = true;
>> >
>> > if (va_size == 32 || stride == 9) {
>> > /* AArch32 or 4KB pages */
>> > @@ -6818,9 +6818,17 @@ static bool get_phys_addr_lpae(CPUARMState *env,
>> > target_ulong address,
>> > level = 3 - startlevel;
>> > }
>> >
>> > -/* Check that the starting level is valid. */
>> > -ok = check_s2_startlevel(cpu, va_size == 64, level,
>> > - inputsize, stride, pamax);
>> > +if (va_size == 64 &&
>> > +inputsize > pamax &&
>> > +(arm_el_is_aa64(env, 1) || inputsize > 40)) {
>>
>> If va_size == 64 doesn't that imply arm_el_is_aa64(env, 1)? Looking
>> further up the function it seems that is what sets va_size in the first
>> place. I think that makes the inputsize > 40 check redundant.
>
> va_size == 64 is true if the EL corresponding to the translation _regime_
> is in 64 bit mode (in this case EL2).
>
> EL1 may very well be in 32bit mode.
Ahh yes, I missed that on the first reading. I think it might be clearer
when reading the code to have the:
bool is_aarch64_regime = (va_size == 64);
And use that to make it clear. And then comment on later check that it's
incompatible with EL1 being aarch32.
>
>>
>> > +/* We have multiple choices but choose to fault. */
>> > +ok = false;
>> > +}
>> > +if (ok) {
>> > +/* Check that the starting level is valid. */
>> > +ok = check_s2_startlevel(cpu, va_size == 64, level,
>> > + inputsize, stride, pamax);
>> > +}
>> > if (!ok) {
>> > /* AArch64 reports these as level 0 faults.
>> > * AArch32 reports these as level 1 faults.
>>
>> I'm not a fan of the ok = true / ok = false / ok =
>> check_s2_start_level() / if (!ok) ping-pong here as it is hard to
>> follow. I'm not sure how you could make it cleaner to follow though.
>> Maybe something like:
>>
>> /* For stage 2 translations the starting level is specified by the
>> * VTCR_EL2.SL0 field (whose interpretation depends on the page size)
>> */
>> int startlevel = extract32(tcr->raw_tcr, 6, 2);
>> unsigned int pamax = arm_pamax(cpu);
>> bool is_aarch64_regime = (va_size == 64);
>> bool ok;
>>
>> if (va_size == 32 || stride == 9) {
>> /* AArch32 or 4KB pages */
>> level = 2 - startlevel;
>> } else {
>> /* 16KB or 64KB pages */
>> level = 3 - startlevel;
>> }
>>
>> if (is_aarch64_regime &&
>> inputsize > pamax) {
>> /* We have multiple choices but choose to fault. */
>> ok = false;
>> } else {
>> /* Check that the starting level is valid. */
>> ok = check_s2_startlevel(cpu, is_aarch64_regime, level,
>> inputsize, stride, pamax);
>> }
>> if (!ok) {
>> /* AArch64 reports these as level 0 faults.
>> * AArch32 reports these as level 1 faults.
>> */
>> level = is_aarch64_regime ? 0 : 1;
>> fault_type = translation_fault;
>> goto do_fault;
>> }
>>
>> But I'm wondering if it just makes more sense to push the:
>>
>> is_aarch64_regime && inputsize > pamax
>>
>> Check into check_s2_startlevel? Then you could just have a simple call
>> which succeeds or falls through to a fault?
>
> Yeah, I guess we could rename check_s2_startlevel to something more generic
> and move all the checks there. I don't feel very strongly about either way...
I think it would be cleaner to follow. get_phys_addr_lpae is already a
bit of a monster so the less conditions to keep track of while reading
it the better IMHO.
> Thanks,
> Edgar
>
>
>
>>
>> /* Check that the starting level is valid. */
>> if (!check_s2_startlevel(cpu, is_aarch64_regime, level,
>> inputsize, stride, pamax) ){
>>
Re: [Qemu-devel] [PATCH v2 09/13] block: Add two dirty bitmap getters
On 20.01.2016 09:11, Fam Zheng wrote:
For dirty bitmap users to get the size and the name of a
BdrvDirtyBitmap.
Signed-off-by: Fam Zheng
Reviewed-by: John Snow
---
block/dirty-bitmap.c | 10 ++
include/block/dirty-bitmap.h | 2 ++
2 files changed, 12 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index d75dcf7..de143f1 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -153,6 +153,16 @@ void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
hbitmap_reset(bitmap->meta, sector, nb_sectors);
}
+int64_t bdrv_dirty_bitmap_size(BdrvDirtyBitmap *bitmap)
+{
+return bitmap->size;
+}
+
+const char *bdrv_dirty_bitmap_name(BdrvDirtyBitmap *bitmap)
+{
+return bitmap->name;
+}
Please, s/BdrvDirtyBitmap */const BdrvDirtyBitmap */
+
bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
{
return bitmap->successor;
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index d9b281a..8c29c3e 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -32,6 +32,8 @@ uint32_t bdrv_get_default_bitmap_granularity(BlockDriverState
*bs);
uint32_t bdrv_dirty_bitmap_granularity(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_enabled(BdrvDirtyBitmap *bitmap);
bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap);
+const char *bdrv_dirty_bitmap_name(BdrvDirtyBitmap *bitmap);
+int64_t bdrv_dirty_bitmap_size(BdrvDirtyBitmap *bitmap);
DirtyBitmapStatus bdrv_dirty_bitmap_status(BdrvDirtyBitmap *bitmap);
int bdrv_get_dirty(BlockDriverState *bs, BdrvDirtyBitmap *bitmap, int64_t
sector);
void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
Re: [Qemu-devel] [PATCH v1] kvm/x86: Hyper-V tsc page setup
On 22/01/2016 12:31, Andrey Smetanin wrote: >> >> Sometimes the TSC is detected to be unstable and Linux switches to >> another clocksource. At least in that case you can get a write to the >> TSC page while the guest is running. > Sorry, now I got it, you mean host TSC is unstable and we should mark > guest tsc page invalid. Now I understand please ignore my prev. message. No problem. Anyhow yes, this is what I meant: a host write to the TSC page, not a guest write to the TSC page MSR. Usually it happens only at migration time to update the sequence---which I believe your patch wasn't doing either. But if we tie TSC page updates to kvm_gen_update_masterclock, we get that for free when the migration destination calls the KVM_SET_CLOCK ioctl. Paolo
Re: [Qemu-devel] [PATCH v2 08/13] block: Support meta dirty bitmap
On 20.01.2016 09:11, Fam Zheng wrote:
The added group of operations enables tracking of the changed bits in
the dirty bitmap.
Signed-off-by: Fam Zheng
---
block/dirty-bitmap.c | 51
include/block/dirty-bitmap.h | 9
2 files changed, 60 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index bd7758b..d75dcf7 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -37,6 +37,7 @@
*/
struct BdrvDirtyBitmap {
HBitmap *bitmap;/* Dirty sector bitmap implementation */
+HBitmap *meta; /* Meta dirty bitmap */
BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */
char *name; /* Optional non-empty unique ID */
int64_t size; /* Size of the bitmap (Number of sectors) */
@@ -102,6 +103,56 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState
*bs,
return bitmap;
}
+/* bdrv_create_meta_dirty_bitmap
+ *
+ * Create a meta dirty bitmap that tracks the changes of bits in @bitmap. I.e.
+ * when a dirty status bit in @bitmap is changed (either from reset to set or
+ * the other way around), its respective meta dirty bitmap bit will be marked
+ * dirty as well.
+ *
+ * @bitmap: the block dirty bitmap for which to create a meta dirty bitmap.
+ * @granularity: how many bytes of bitmap data does each bit in the meta bitmap
+ * track.
+ */
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+ int granularity)
+{
+assert(!bitmap->meta);
+bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
+ BDRV_SECTOR_SIZE * BITS_PER_BYTE);
+}
+
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+assert(bitmap->meta);
+hbitmap_free_meta(bitmap->bitmap);
+bitmap->meta = NULL;
+}
+
+int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors)
+{
+uint64_t i;
+int gran = bdrv_dirty_bitmap_granularity(bitmap) >> BDRV_SECTOR_BITS;
+
+/* To optimize: we can make hbitmap to internally check the range in a
+ * coarse level, or at least do it word by word. */
+for (i = sector; i < sector + nb_sectors; i += gran) {
+if (hbitmap_get(bitmap->meta, i)) {
+return true;
+}
+}
+return false;
+}
+
+void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors)
+{
+hbitmap_reset(bitmap->meta, sector, nb_sectors);
+}
+
bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
{
return bitmap->successor;
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 120bac6..d9b281a 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -9,6 +9,9 @@ BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs,
uint32_t granularity,
const char *name,
Error **errp);
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+ int granularity);
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap,
Error **errp);
@@ -35,6 +38,12 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
+int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors);
+void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors);
BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
uint64_t first_sector);
void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
In my migration series I need iterators, get granularity, and something
like hbitmap_count for meta bitmaps. You can add them here if you want,
or I can add them in my series.
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
Re: [Qemu-devel] [PATCH v2 8/9] ipmi: add ACPI power and GUID commands
On 01/22/2016 12:24 PM, Greg Kurz wrote:
> On Thu, 21 Jan 2016 18:18:53 +0100
> Cédric Le Goater wrote:
>
>> Signed-off-by: Cédric Le Goater
>> ---
>>
>> Changes since v1:
>> - added ACPI to command names.
>>
>> hw/ipmi/ipmi_bmc_sim.c | 49
>> +
>> 1 file changed, 49 insertions(+)
>>
>> diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
>> index e882af3f1b40..53c75cb21c1a 100644
>> --- a/hw/ipmi/ipmi_bmc_sim.c
>> +++ b/hw/ipmi/ipmi_bmc_sim.c
>> @@ -25,6 +25,7 @@
>> #include
>> #include
>> #include
>> +#include "sysemu/sysemu.h"
>> #include "qemu/timer.h"
>> #include "hw/ipmi/ipmi.h"
>> #include "qemu/error-report.h"
>> @@ -51,6 +52,9 @@
>> #define IPMI_CMD_GET_DEVICE_ID0x01
>> #define IPMI_CMD_COLD_RESET 0x02
>> #define IPMI_CMD_WARM_RESET 0x03
>> +#define IPMI_CMD_SET_ACPI_POWER_STATE 0x06
>> +#define IPMI_CMD_GET_ACPI_POWER_STATE 0x07
>> +#define IPMI_CMD_GET_DEVICE_GUID 0x08
>> #define IPMI_CMD_RESET_WATCHDOG_TIMER 0x22
>> #define IPMI_CMD_SET_WATCHDOG_TIMER 0x24
>> #define IPMI_CMD_GET_WATCHDOG_TIMER 0x25
>> @@ -200,6 +204,9 @@ struct IPMIBmcSim {
>>
>> uint8_t restart_cause;
>>
>> +uint8_t acpi_power_state[2];
>> +uint8_t uuid[16];
>> +
>> IPMISel sel;
>> IPMISdr sdr;
>> IPMISensor sensors[MAX_SENSORS];
>> @@ -828,6 +835,36 @@ static void warm_reset(IPMIBmcSim *ibs,
>> k->reset(s, false);
>> }
>> }
>> +static void set_acpi_power_state(IPMIBmcSim *ibs,
>> + uint8_t *cmd, unsigned int cmd_len,
>> + uint8_t *rsp, unsigned int *rsp_len,
>> + unsigned int max_rsp_len)
>> +{
>> +IPMI_CHECK_CMD_LEN(4);
>> +ibs->acpi_power_state[0] = cmd[2];
>> +ibs->acpi_power_state[1] = cmd[3];
>> +}
>> +
>> +static void get_acpi_power_state(IPMIBmcSim *ibs,
>> + uint8_t *cmd, unsigned int cmd_len,
>> + uint8_t *rsp, unsigned int *rsp_len,
>> + unsigned int max_rsp_len)
>> +{
>> +IPMI_ADD_RSP_DATA(ibs->acpi_power_state[0]);
>> +IPMI_ADD_RSP_DATA(ibs->acpi_power_state[1]);
>> +}
>> +
>> +static void get_device_guid(IPMIBmcSim *ibs,
>> + uint8_t *cmd, unsigned int cmd_len,
>> + uint8_t *rsp, unsigned int *rsp_len,
>> + unsigned int max_rsp_len)
>> +{
>> +unsigned int i;
>> +
>> +for (i = 0; i < 16; i++) {
>> +IPMI_ADD_RSP_DATA(ibs->uuid[i]);
>> +}
>> +}
>>
>> static void set_bmc_global_enables(IPMIBmcSim *ibs,
>> uint8_t *cmd, unsigned int cmd_len,
>> @@ -1609,6 +1646,9 @@ static const IPMICmdHandler app_cmds[] = {
>> [IPMI_CMD_GET_DEVICE_ID] = get_device_id,
>> [IPMI_CMD_COLD_RESET] = cold_reset,
>> [IPMI_CMD_WARM_RESET] = warm_reset,
>> +[IPMI_CMD_SET_ACPI_POWER_STATE] = set_acpi_power_state,
>> +[IPMI_CMD_GET_ACPI_POWER_STATE] = get_acpi_power_state,
>> +[IPMI_CMD_GET_DEVICE_GUID] = get_device_guid,
>> [IPMI_CMD_SET_BMC_GLOBAL_ENABLES] = set_bmc_global_enables,
>> [IPMI_CMD_GET_BMC_GLOBAL_ENABLES] = get_bmc_global_enables,
>> [IPMI_CMD_CLR_MSG_FLAGS] = clr_msg_flags,
>> @@ -1734,6 +1774,15 @@ static void ipmi_sim_init(Object *obj)
>> i += len;
>> }
>>
>> +ibs->acpi_power_state[0] = 0;
>> +ibs->acpi_power_state[1] = 0;
>> +
>> +if (qemu_uuid_set) {
>> +memcpy(&ibs->uuid, qemu_uuid, 16);
>> +} else {
>> +memset(&ibs->uuid, 0, 16);
>> +}
>> +
>
> Sorry if this is a dumb question: why does the VM's UUID gets copied here ?
>From the specs (20.8 Get Device GUID Command), the command needs to return
a GUID (Globally Unique ID), or UUID, that should never change over the
lifetime of the device.
qemu_uuid looked like a good candidate to start with but we could use a
property also if needed.
Thanks,
C.
Re: [Qemu-devel] [PATCH v1] kvm/x86: Hyper-V tsc page setup
On 01/22/2016 02:53 PM, Paolo Bonzini wrote: On 22/01/2016 12:31, Andrey Smetanin wrote: Sometimes the TSC is detected to be unstable and Linux switches to another clocksource. At least in that case you can get a write to the TSC page while the guest is running. Sorry, now I got it, you mean host TSC is unstable and we should mark guest tsc page invalid. Now I understand please ignore my prev. message. No problem. Anyhow yes, this is what I meant: a host write to the TSC page, not a guest write to the TSC page MSR. Usually it happens only at migration time to update the sequence---which I believe your patch wasn't doing either. QEMU saves address of page inside ->msr_hv_tsc, so at restore QEMU sets corresponding MSR and KVM setup's tsc page again. So migration should able to work. But if we tie TSC page updates to kvm_gen_update_masterclock, we get that for free when the migration destination calls the KVM_SET_CLOCK ioctl. Paolo
Re: [Qemu-devel] [PATCH] seabios: fix submodule
On 22 January 2016 at 10:59, Gerd Hoffmann wrote: > Commit "36f96c4 target-i386: Add support to migrate vcpu's TSC rate" > updates roms/seabios, appearently by mistake. Revert this. > > Signed-off-by: Gerd Hoffmann > --- > roms/seabios | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/roms/seabios b/roms/seabios > index 33fbe13..01a84be 16 > --- a/roms/seabios > +++ b/roms/seabios > @@ -1 +1 @@ > -Subproject commit 33fbe13a3e2a01e0ba1087a8feed801a0451db21 > +Subproject commit 01a84bea2d28a19d2405c1ecac4bdef17683cc0c > -- > 1.8.3.1 Whoops. Unfortunately git can make it easy to accidentally include submodule updates in patches or pull requests, and I didn't catch this one on applying the pullreq either. I've applied this patch to master to correct the error. thanks -- PMM
Re: [Qemu-devel] [PATCH v2 08/13] block: Support meta dirty bitmap
On 22.01.2016 14:58, Vladimir Sementsov-Ogievskiy wrote:
On 20.01.2016 09:11, Fam Zheng wrote:
The added group of operations enables tracking of the changed bits in
the dirty bitmap.
Signed-off-by: Fam Zheng
---
block/dirty-bitmap.c | 51
include/block/dirty-bitmap.h | 9
2 files changed, 60 insertions(+)
diff --git a/block/dirty-bitmap.c b/block/dirty-bitmap.c
index bd7758b..d75dcf7 100644
--- a/block/dirty-bitmap.c
+++ b/block/dirty-bitmap.c
@@ -37,6 +37,7 @@
*/
struct BdrvDirtyBitmap {
HBitmap *bitmap;/* Dirty sector bitmap
implementation */
+HBitmap *meta; /* Meta dirty bitmap */
BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen
status */
char *name; /* Optional non-empty unique ID */
int64_t size; /* Size of the bitmap (Number of
sectors) */
@@ -102,6 +103,56 @@ BdrvDirtyBitmap
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
return bitmap;
}
+/* bdrv_create_meta_dirty_bitmap
+ *
+ * Create a meta dirty bitmap that tracks the changes of bits in
@bitmap. I.e.
+ * when a dirty status bit in @bitmap is changed (either from reset
to set or
+ * the other way around), its respective meta dirty bitmap bit will
be marked
+ * dirty as well.
+ *
+ * @bitmap: the block dirty bitmap for which to create a meta dirty
bitmap.
+ * @granularity: how many bytes of bitmap data does each bit in the
meta bitmap
+ * track.
+ */
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+ int granularity)
+{
+assert(!bitmap->meta);
+bitmap->meta = hbitmap_create_meta(bitmap->bitmap,
+ BDRV_SECTOR_SIZE *
BITS_PER_BYTE);
+}
+
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap)
+{
+assert(bitmap->meta);
+hbitmap_free_meta(bitmap->bitmap);
+bitmap->meta = NULL;
+}
+
+int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors)
+{
+uint64_t i;
+int gran = bdrv_dirty_bitmap_granularity(bitmap) >>
BDRV_SECTOR_BITS;
+
+/* To optimize: we can make hbitmap to internally check the
range in a
+ * coarse level, or at least do it word by word. */
+for (i = sector; i < sector + nb_sectors; i += gran) {
+if (hbitmap_get(bitmap->meta, i)) {
+return true;
+}
+}
+return false;
+}
+
+void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t
sector,
+ int nb_sectors)
+{
+hbitmap_reset(bitmap->meta, sector, nb_sectors);
+}
+
bool bdrv_dirty_bitmap_frozen(BdrvDirtyBitmap *bitmap)
{
return bitmap->successor;
diff --git a/include/block/dirty-bitmap.h b/include/block/dirty-bitmap.h
index 120bac6..d9b281a 100644
--- a/include/block/dirty-bitmap.h
+++ b/include/block/dirty-bitmap.h
@@ -9,6 +9,9 @@ BdrvDirtyBitmap
*bdrv_create_dirty_bitmap(BlockDriverState *bs,
uint32_t granularity,
const char *name,
Error **errp);
+void bdrv_create_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap,
+ int granularity);
+void bdrv_release_meta_dirty_bitmap(BdrvDirtyBitmap *bitmap);
int bdrv_dirty_bitmap_create_successor(BlockDriverState *bs,
BdrvDirtyBitmap *bitmap,
Error **errp);
@@ -35,6 +38,12 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap,
int64_t cur_sector, int nr_sectors);
+int bdrv_dirty_bitmap_get_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t sector,
+ int nb_sectors);
+void bdrv_dirty_bitmap_reset_meta(BlockDriverState *bs,
+ BdrvDirtyBitmap *bitmap, int64_t
sector,
+ int nb_sectors);
BdrvDirtyBitmapIter *bdrv_dirty_iter_new(BdrvDirtyBitmap *bitmap,
uint64_t first_sector);
void bdrv_dirty_iter_free(BdrvDirtyBitmapIter *iter);
In my migration series I need iterators, get granularity, and
something like hbitmap_count for meta bitmaps. You can add them here
if you want, or I can add them in my series.
Oh, sorry, I can't. I don't know what to do with active_iterators in
this case, as it relates to bitmap, not to meta.. It is not as easy as
it seemed. So, this should be resolved in these series.
--
Best regards,
Vladimir
* now, @virtuozzo.com instead of @parallels.com. Sorry for this inco
Re: [Qemu-devel] [PATCH v2 00/13] Dirty bitmap changes for migration/persistence work
On 20.01.2016 19:13, Vladimir Sementsov-Ogievskiy wrote: I'll try to rebase my series on this and run tests. Now failed because of iterators for meta bitmaps are unsupported, see answers to [PATCH v2 08/13]. Wait for v3. On 20.01.2016 09:11, Fam Zheng wrote: v2: Various changes addressing John's and Vladimir's comments: [02/13] typedefs: Add BdrvDirtyBitmap Skip HBitmapIter because we'll hide it soon. [John] [03/13] block: Move block dirty bitmap code to separate files [04/13] block: Remove unused typedef of BlockDriverDirtyHandler [05/13] block: Hide HBitmap in block dirty bitmap interface Add assert in bdrv_dirty_bitmap_truncate(). [John] Add John's rev-by. [06/13] HBitmap: Introduce "meta" bitmap to track bit changes Caller of hbitmap_create_meta() frees it with hbitmap_free_meta(). [John, Vladimir] [07/13] tests: Add test code for meta bitmap Add John's rev-by. [08/13] block: Support meta dirty bitmap Release the meta dirty bitmap in bdrv_release_dirty_bitmap(). [09/13] block: Add two dirty bitmap getters [10/13] block: Assert that bdrv_release_dirty_bitmap succeeded Add John's rev-by. [11/13] hbitmap: serialization Fix comment for hbitmap_serialization_granularity() and hbitmap_deserialize_part(). [John] Document @finish in hbitmap_deserialize_zeroes more clearly. Fixed granularity in hbitmap_serialization_granularity(). [Vladimir] Tweak the assertion in serialization_chunk. [Vladimir] cpu_to_leXXs -> leXX_to_cpus in hbitmap_deserialize_part. [Vladimir] Fix typo in serialization_chunk() comments. [John] [12/13] block: BdrvDirtyBitmap serialization interface [13/13] tests: Add test code for hbitmap serialization Two major features are added to block dirty bitmap (and underlying HBitmap) in this series: meta bitmap and serialization, together with all other supportive patches. Both operations are common in dirty bitmap migration and persistence: they need to find whether and which part of the dirty bitmap in question has changed with meta dirty bitmap, and they need to write it to the target with serialization. Fam Zheng (11): backup: Use Bitmap to replace "s->bitmap" typedefs: Add BdrvDirtyBitmap block: Move block dirty bitmap code to separate files block: Remove unused typedef of BlockDriverDirtyHandler block: Hide HBitmap in block dirty bitmap interface HBitmap: Introduce "meta" bitmap to track bit changes tests: Add test code for meta bitmap block: Support meta dirty bitmap block: Add two dirty bitmap getters block: Assert that bdrv_release_dirty_bitmap succeeded tests: Add test code for hbitmap serialization Vladimir Sementsov-Ogievskiy (2): hbitmap: serialization block: BdrvDirtyBitmap serialization interface block.c | 339 - block/Makefile.objs | 2 +- block/backup.c | 25 ++- block/dirty-bitmap.c | 492 +++ block/mirror.c | 14 +- include/block/block.h| 40 +--- include/block/dirty-bitmap.h | 71 +++ include/qemu/hbitmap.h | 95 + include/qemu/typedefs.h | 2 + tests/test-hbitmap.c | 255 ++ util/hbitmap.c | 201 -- 11 files changed, 1126 insertions(+), 410 deletions(-) create mode 100644 block/dirty-bitmap.c create mode 100644 include/block/dirty-bitmap.h -- Best regards, Vladimir * now, @virtuozzo.com instead of @parallels.com. Sorry for this inconvenience.
[Qemu-devel] [PATCH 4/6] scripts/dump-guest-memory.py: Cleanup functions
Increase readability by adding newlines and comments, as well as
removing wrong whitespaces and C style braces around conditionals and
loops.
Reviewed-by: Laszlo Ersek
Signed-off-by: Janosch Frank
---
scripts/dump-guest-memory.py | 75 +---
1 file changed, 50 insertions(+), 25 deletions(-)
diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index bb4ca8e..2cf7365 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -69,35 +69,60 @@ ELF64_PHDR = ("I" # p_type
)
def int128_get64(val):
-assert (val["hi"] == 0)
+"""Returns low 64bit part of Int128 struct."""
+
+assert val["hi"] == 0
return val["lo"]
+
def qlist_foreach(head, field_str):
+"""Generator for qlists."""
+
var_p = head["lh_first"]
-while (var_p != 0):
+while var_p != 0:
var = var_p.dereference()
-yield var
var_p = var[field_str]["le_next"]
+yield var
+
def qemu_get_ram_block(ram_addr):
+"""Returns the RAMBlock struct to which the given address belongs."""
+
ram_blocks = gdb.parse_and_eval("ram_list.blocks")
+
for block in qlist_foreach(ram_blocks, "next"):
-if (ram_addr - block["offset"] < block["used_length"]):
+if (ram_addr - block["offset"]) < block["used_length"]:
return block
+
raise gdb.GdbError("Bad ram offset %x" % ram_addr)
+
def qemu_get_ram_ptr(ram_addr):
+"""Returns qemu vaddr for given guest physical address."""
+
block = qemu_get_ram_block(ram_addr)
return block["host"] + (ram_addr - block["offset"])
-def memory_region_get_ram_ptr(mr):
-if (mr["alias"] != 0):
-return (memory_region_get_ram_ptr(mr["alias"].dereference()) +
-mr["alias_offset"])
-return qemu_get_ram_ptr(mr["ram_addr"] & TARGET_PAGE_MASK)
+
+def memory_region_get_ram_ptr(memory_region):
+if memory_region["alias"] != 0:
+return (memory_region_get_ram_ptr(memory_region["alias"].dereference())
++ memory_region["alias_offset"])
+
+return qemu_get_ram_ptr(memory_region["ram_addr"] & TARGET_PAGE_MASK)
+
def get_guest_phys_blocks():
+"""Returns a list of ram blocks.
+
+Each block entry contains:
+'target_start': guest block phys start address
+'target_end': guest block phys end address
+'host_addr':qemu vaddr of the block's start
+"""
+
guest_phys_blocks = []
+
print("guest RAM blocks:")
print("target_start target_end host_addrmessage "
"count")
@@ -111,29 +136,29 @@ def get_guest_phys_blocks():
# compatibility. Otherwise range doesn't cast the value itself and
# breaks.
for cur in range(int(current_map["nr"])):
-flat_range = (current_map["ranges"] + cur).dereference()
-mr = flat_range["mr"].dereference()
+flat_range = (current_map["ranges"] + cur).dereference()
+memory_region = flat_range["mr"].dereference()
# we only care about RAM
-if (not mr["ram"]):
+if not memory_region["ram"]:
continue
section_size = int128_get64(flat_range["addr"]["size"])
target_start = int128_get64(flat_range["addr"]["start"])
-target_end = target_start + section_size
-host_addr= (memory_region_get_ram_ptr(mr) +
-flat_range["offset_in_region"])
+target_end = target_start + section_size
+host_addr = (memory_region_get_ram_ptr(memory_region)
+ + flat_range["offset_in_region"])
predecessor = None
# find continuity in guest physical address space
-if (len(guest_phys_blocks) > 0):
+if len(guest_phys_blocks) > 0:
predecessor = guest_phys_blocks[-1]
predecessor_size = (predecessor["target_end"] -
predecessor["target_start"])
# the memory API guarantees monotonically increasing
# traversal
-assert (predecessor["target_end"] <= target_start)
+assert predecessor["target_end"] <= target_start
# we want continuity in both guest-physical and
# host-virtual memory
@@ -141,11 +166,11 @@ def get_guest_phys_blocks():
predecessor["host_addr"] + predecessor_size != host_addr):
predecessor = None
-if (predecessor is None):
+if predecessor is None:
# isolated mapping, add it to the list
guest_phys_blocks.append({"target_start": target_start,
- "target_end" : target_end,
- "host_addr" : host_addr})
+ "target_end": target_end,
+ "host_addr":host_addr})
message = "added"
else:
# expand predecessor until @target
[Qemu-devel] [PATCH 3/6] scripts/dump-guest-memory.py: Improve python 3 compatibility
This commit does not make the script python 3 compatible, it is a
preparation that fixes the easy and common incompatibilities.
Print is a function in python 3 and therefore needs braces around its
arguments.
Range does not cast a gdb.Value object to int in python 3, we have to
do it ourselves.
Reviewed-by: Laszlo Ersek
Signed-off-by: Janosch Frank
---
scripts/dump-guest-memory.py | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index d0b927a..bb4ca8e 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -98,15 +98,19 @@ def memory_region_get_ram_ptr(mr):
def get_guest_phys_blocks():
guest_phys_blocks = []
-print "guest RAM blocks:"
-print ("target_start target_end host_addrmessage "
- "count")
-print (" --- "
- "-")
+print("guest RAM blocks:")
+print("target_start target_end host_addrmessage "
+ "count")
+print(" --- "
+ "-")
current_map_p = gdb.parse_and_eval("address_space_memory.current_map")
current_map = current_map_p.dereference()
-for cur in range(current_map["nr"]):
+
+# Conversion to int is needed for python 3
+# compatibility. Otherwise range doesn't cast the value itself and
+# breaks.
+for cur in range(int(current_map["nr"])):
flat_range = (current_map["ranges"] + cur).dereference()
mr = flat_range["mr"].dereference()
@@ -149,9 +153,9 @@ def get_guest_phys_blocks():
predecessor["target_end"] = target_end
message = "joined"
-print ("%016x %016x %016x %-7s %5u" %
- (target_start, target_end, host_addr.cast(UINTPTR_T),
-message, len(guest_phys_blocks)))
+print("%016x %016x %016x %-7s %5u" %
+ (target_start, target_end, host_addr.cast(UINTPTR_T),
+ message, len(guest_phys_blocks)))
return guest_phys_blocks
@@ -311,8 +315,8 @@ shape and this command should mostly work."""
for block in self.guest_phys_blocks:
cur = block["host_addr"]
left = block["target_end"] - block["target_start"]
-print ("dumping range at %016x for length %016x" %
- (cur.cast(UINTPTR_T), left))
+print("dumping range at %016x for length %016x" %
+ (cur.cast(UINTPTR_T), left))
while (left > 0):
chunk_size = min(TARGET_PAGE_SIZE, left)
chunk = qemu_core.read_memory(cur, chunk_size)
--
2.3.0
[Qemu-devel] [PATCH 5/6] scripts/dump-guest-memory.py: Introduce multi-arch support
By modelling the ELF with ctypes we not only gain full python 3
support but can also create dumps for different architectures more easily.
Tested-by: Andrew Jones
Acked-by: Laszlo Ersek
Signed-off-by: Janosch Frank
---
scripts/dump-guest-memory.py | 486 ---
1 file changed, 322 insertions(+), 164 deletions(-)
diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index 2cf7365..7acce65 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -6,6 +6,7 @@
#
# Authors:
# Laszlo Ersek
+# Janosch Frank
#
# This work is licensed under the terms of the GNU GPL, version 2 or later. See
# the COPYING file in the top-level directory.
@@ -15,58 +16,303 @@
# "help data" summary), and it should match how other help texts look in
# gdb.
-import struct
+import ctypes
UINTPTR_T = gdb.lookup_type("uintptr_t")
TARGET_PAGE_SIZE = 0x1000
TARGET_PAGE_MASK = 0xF000
-# Various ELF constants
-EM_X86_64 = 62# AMD x86-64 target machine
-ELFDATA2LSB = 1 # little endian
-ELFCLASS64 = 2
-ELFMAG = "\x7FELF"
-EV_CURRENT = 1
-ET_CORE = 4
-PT_LOAD = 1
-PT_NOTE = 4
-
# Special value for e_phnum. This indicates that the real number of
# program headers is too large to fit into e_phnum. Instead the real
# value is in the field sh_info of section 0.
PN_XNUM = 0x
-# Format strings for packing and header size calculation.
-ELF64_EHDR = ("4s" # e_ident/magic
- "B" # e_ident/class
- "B" # e_ident/data
- "B" # e_ident/version
- "B" # e_ident/osabi
- "8s" # e_ident/pad
- "H" # e_type
- "H" # e_machine
- "I" # e_version
- "Q" # e_entry
- "Q" # e_phoff
- "Q" # e_shoff
- "I" # e_flags
- "H" # e_ehsize
- "H" # e_phentsize
- "H" # e_phnum
- "H" # e_shentsize
- "H" # e_shnum
- "H" # e_shstrndx
- )
-ELF64_PHDR = ("I" # p_type
- "I" # p_flags
- "Q" # p_offset
- "Q" # p_vaddr
- "Q" # p_paddr
- "Q" # p_filesz
- "Q" # p_memsz
- "Q" # p_align
- )
+EV_CURRENT = 1
+
+ELFCLASS32 = 1
+ELFCLASS64 = 2
+
+ELFDATA2LSB = 1
+ELFDATA2MSB = 2
+
+ET_CORE = 4
+
+PT_LOAD = 1
+PT_NOTE = 4
+
+EM_386 = 3
+EM_PPC = 20
+EM_PPC64 = 21
+EM_S390 = 22
+EM_AARCH = 183
+EM_X86_64 = 62
+
+class ELF(object):
+"""Representation of a ELF file."""
+
+def __init__(self, arch):
+self.ehdr = None
+self.notes = []
+self.segments = []
+self.notes_size = 0
+self.endianess = None
+self.elfclass = ELFCLASS64
+
+if arch == 'aarch64-le':
+self.endianess = ELFDATA2LSB
+self.elfclass = ELFCLASS64
+self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+self.ehdr.e_machine = EM_AARCH
+
+elif arch == 'aarch64-be':
+self.endianess = ELFDATA2MSB
+self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+self.ehdr.e_machine = EM_AARCH
+
+elif arch == 'X86_64':
+self.endianess = ELFDATA2LSB
+self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+self.ehdr.e_machine = EM_X86_64
+
+elif arch == '386':
+self.endianess = ELFDATA2LSB
+self.elfclass = ELFCLASS32
+self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+self.ehdr.e_machine = EM_386
+
+elif arch == 's390':
+self.endianess = ELFDATA2MSB
+self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+self.ehdr.e_machine = EM_S390
+
+elif arch == 'ppc64-le':
+self.endianess = ELFDATA2LSB
+self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+self.ehdr.e_machine = EM_PPC64
+
+elif arch == 'ppc64-be':
+self.endianess = ELFDATA2MSB
+self.ehdr = get_arch_ehdr(self.endianess, self.elfclass)
+self.ehdr.e_machine = EM_PPC64
+
+else:
+raise gdb.GdbError("No valid arch type specified.\n"
+ "Currently supported types:\n"
+ "aarch64-be, aarch64-le, X86_64, 386, s390, "
+ "ppc64-be, ppc64-le")
+
+self.add_segment(PT_NOTE, 0, 0)
+
+def add_note(self, n_name, n_desc, n_type):
+"""Adds a note to the ELF."""
+
+note = get_arch_note(self.endianess, len(n_name), len(n_desc))
+note.n_namesz = len(n_name) + 1
+note.n_descsz = len(n_desc)
+note.n_name = n_name.encode()
+note.n_type = n_type
+
+# Desc needs to be 4 byte aligned (although the 64bit spec
+# specifies 8 byte). Wh
[Qemu-devel] [PATCH 0/6] scripts/dump-guest-memory.py: Add multi-arch support
The dump guest memory script for extracting a Linux core from a qemu core is currently limited to amd64 and python 2. With this series we add support for python 3 (while maintaining python 2 support) and add the possibility to extract dumps from VMs with the most common architectures. Tested on X86_64, s390 and Aarch64-le (thanks to Andrew Jones). RFC to PATCH: Tested on X86_64 and Aarch64-le. Fix wrongly indented return in get_guest_phys_blocks(). Fix wrong indent for predecessor_size that happened when copying. Fix inconsistent line wrapping. Fix python 3 compatibility comment being added in the wrong commit. Fix docstrings that get displayed by gdb. Replaced ceil(len_desc / 4.0) with ((len_desc + 3) // 4) Added patch that fixes module docstring to account for moved gdb help string. Janosch Frank (6): scripts/dump-guest-memory.py: Move constants to the top scripts/dump-guest-memory.py: Make methods functions scripts/dump-guest-memory.py: Improve python 3 compatibility scripts/dump-guest-memory.py: Cleanup functions scripts/dump-guest-memory.py: Introduce multi-arch support scripts/dump-guest-memory.py: Fix module docstring scripts/dump-guest-memory.py | 754 +++ 1 file changed, 472 insertions(+), 282 deletions(-) -- 2.3.0
[Qemu-devel] [PATCH 6/6] scripts/dump-guest-memory.py: Fix module docstring
The module docstring is changed into a multi-line comment to comply with pep 257. The comment about the docstring that gets used by gdb to print the help is moved to the location of the docstring. Signed-off-by: Janosch Frank --- scripts/dump-guest-memory.py | 35 ++- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py index 7acce65..f274bf8 100644 --- a/scripts/dump-guest-memory.py +++ b/scripts/dump-guest-memory.py @@ -1,20 +1,17 @@ -# This python script adds a new gdb command, "dump-guest-memory". It -# should be loaded with "source dump-guest-memory.py" at the (gdb) -# prompt. -# -# Copyright (C) 2013, Red Hat, Inc. -# -# Authors: -# Laszlo Ersek -# Janosch Frank -# -# This work is licensed under the terms of the GNU GPL, version 2 or later. See -# the COPYING file in the top-level directory. -# -# The leading docstring doesn't have idiomatic Python formatting. It is -# printed by gdb's "help" command (the first line is printed in the -# "help data" summary), and it should match how other help texts look in -# gdb. +""" +This python script adds a new gdb command, "dump-guest-memory". It +should be loaded with "source dump-guest-memory.py" at the (gdb) +prompt. + +Copyright (C) 2013, Red Hat, Inc. + +Authors: + Laszlo Ersek + Janosch Frank + +This work is licensed under the terms of the GNU GPL, version 2 or later. See +the COPYING file in the top-level directory. +""" import ctypes @@ -431,6 +428,10 @@ def get_guest_phys_blocks(): return guest_phys_blocks +# The leading docstring doesn't have idiomatic Python formatting. It is +# printed by gdb's "help" command (the first line is printed in the +# "help data" summary), and it should match how other help texts look in +# gdb. class DumpGuestMemory(gdb.Command): """Extract guest vmcore from qemu process coredump. -- 2.3.0
[Qemu-devel] [PATCH 1/6] scripts/dump-guest-memory.py: Move constants to the top
The constants bloated the class definition and were therefore moved to
the top.
Reviewed-by: Laszlo Ersek
Signed-off-by: Janosch Frank
---
scripts/dump-guest-memory.py | 126 +--
1 file changed, 63 insertions(+), 63 deletions(-)
diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index 08796ff..e49c835 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -17,6 +17,55 @@
import struct
+TARGET_PAGE_SIZE = 0x1000
+TARGET_PAGE_MASK = 0xF000
+
+# Various ELF constants
+EM_X86_64 = 62# AMD x86-64 target machine
+ELFDATA2LSB = 1 # little endian
+ELFCLASS64 = 2
+ELFMAG = "\x7FELF"
+EV_CURRENT = 1
+ET_CORE = 4
+PT_LOAD = 1
+PT_NOTE = 4
+
+# Special value for e_phnum. This indicates that the real number of
+# program headers is too large to fit into e_phnum. Instead the real
+# value is in the field sh_info of section 0.
+PN_XNUM = 0x
+
+# Format strings for packing and header size calculation.
+ELF64_EHDR = ("4s" # e_ident/magic
+ "B" # e_ident/class
+ "B" # e_ident/data
+ "B" # e_ident/version
+ "B" # e_ident/osabi
+ "8s" # e_ident/pad
+ "H" # e_type
+ "H" # e_machine
+ "I" # e_version
+ "Q" # e_entry
+ "Q" # e_phoff
+ "Q" # e_shoff
+ "I" # e_flags
+ "H" # e_ehsize
+ "H" # e_phentsize
+ "H" # e_phnum
+ "H" # e_shentsize
+ "H" # e_shnum
+ "H" # e_shstrndx
+ )
+ELF64_PHDR = ("I" # p_type
+ "I" # p_flags
+ "Q" # p_offset
+ "Q" # p_vaddr
+ "Q" # p_paddr
+ "Q" # p_filesz
+ "Q" # p_memsz
+ "Q" # p_align
+ )
+
class DumpGuestMemory(gdb.Command):
"""Extract guest vmcore from qemu process coredump.
@@ -47,62 +96,13 @@ deliberately called abort(), or it was dumped in response
to a signal at
a halfway fortunate point, then its coredump should be in reasonable
shape and this command should mostly work."""
-TARGET_PAGE_SIZE = 0x1000
-TARGET_PAGE_MASK = 0xF000
-
-# Various ELF constants
-EM_X86_64 = 62# AMD x86-64 target machine
-ELFDATA2LSB = 1 # little endian
-ELFCLASS64 = 2
-ELFMAG = "\x7FELF"
-EV_CURRENT = 1
-ET_CORE = 4
-PT_LOAD = 1
-PT_NOTE = 4
-
-# Special value for e_phnum. This indicates that the real number of
-# program headers is too large to fit into e_phnum. Instead the real
-# value is in the field sh_info of section 0.
-PN_XNUM = 0x
-
-# Format strings for packing and header size calculation.
-ELF64_EHDR = ("4s" # e_ident/magic
- "B" # e_ident/class
- "B" # e_ident/data
- "B" # e_ident/version
- "B" # e_ident/osabi
- "8s" # e_ident/pad
- "H" # e_type
- "H" # e_machine
- "I" # e_version
- "Q" # e_entry
- "Q" # e_phoff
- "Q" # e_shoff
- "I" # e_flags
- "H" # e_ehsize
- "H" # e_phentsize
- "H" # e_phnum
- "H" # e_shentsize
- "H" # e_shnum
- "H" # e_shstrndx
- )
-ELF64_PHDR = ("I" # p_type
- "I" # p_flags
- "Q" # p_offset
- "Q" # p_vaddr
- "Q" # p_paddr
- "Q" # p_filesz
- "Q" # p_memsz
- "Q" # p_align
- )
-
def __init__(self):
super(DumpGuestMemory, self).__init__("dump-guest-memory",
gdb.COMMAND_DATA,
gdb.COMPLETE_FILENAME)
self.uintptr_t = gdb.lookup_type("uintptr_t")
-self.elf64_ehdr_le = struct.Struct("<%s" % self.ELF64_EHDR)
-self.elf64_phdr_le = struct.Struct("<%s" % self.ELF64_PHDR)
+self.elf64_ehdr_le = struct.Struct("<%s" % ELF64_EHDR)
+self.elf64_phdr_le = struct.Struct("<%s" % ELF64_PHDR)
def int128_get64(self, val):
assert (val["hi"] == 0)
@@ -130,7 +130,7 @@ shape and this command should mostly work."""
if (mr["alias"] != 0):
return (self.memory_region_get_ram_ptr(mr["alias"].dereference()) +
mr["alias_offset"])
-return self.qemu_get_ram_ptr(mr["ram_addr"] & self.TARGET_PAGE_MASK)
+return self.qemu_get_ram_ptr(mr["ram_addr"] & TARGET_PAGE_MASK)
def guest_phys_blocks_init(self):
self.guest_phys_blocks = []
@@ -198,21 +198,21 @@ shape and this comma
[Qemu-devel] [PATCH 2/6] scripts/dump-guest-memory.py: Make methods functions
The functions dealing with qemu components rarely used parts of the
class, so they were moved out of the class.
As the uintptr_t variable is needed both within and outside the class,
it was made a constant and moved to the top.
Reviewed-by: Laszlo Ersek
Signed-off-by: Janosch Frank
---
scripts/dump-guest-memory.py | 184 ++-
1 file changed, 93 insertions(+), 91 deletions(-)
diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index e49c835..d0b927a 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -17,6 +17,8 @@
import struct
+UINTPTR_T = gdb.lookup_type("uintptr_t")
+
TARGET_PAGE_SIZE = 0x1000
TARGET_PAGE_MASK = 0xF000
@@ -66,6 +68,94 @@ ELF64_PHDR = ("I" # p_type
"Q" # p_align
)
+def int128_get64(val):
+assert (val["hi"] == 0)
+return val["lo"]
+
+def qlist_foreach(head, field_str):
+var_p = head["lh_first"]
+while (var_p != 0):
+var = var_p.dereference()
+yield var
+var_p = var[field_str]["le_next"]
+
+def qemu_get_ram_block(ram_addr):
+ram_blocks = gdb.parse_and_eval("ram_list.blocks")
+for block in qlist_foreach(ram_blocks, "next"):
+if (ram_addr - block["offset"] < block["used_length"]):
+return block
+raise gdb.GdbError("Bad ram offset %x" % ram_addr)
+
+def qemu_get_ram_ptr(ram_addr):
+block = qemu_get_ram_block(ram_addr)
+return block["host"] + (ram_addr - block["offset"])
+
+def memory_region_get_ram_ptr(mr):
+if (mr["alias"] != 0):
+return (memory_region_get_ram_ptr(mr["alias"].dereference()) +
+mr["alias_offset"])
+return qemu_get_ram_ptr(mr["ram_addr"] & TARGET_PAGE_MASK)
+
+def get_guest_phys_blocks():
+guest_phys_blocks = []
+print "guest RAM blocks:"
+print ("target_start target_end host_addrmessage "
+ "count")
+print (" --- "
+ "-")
+
+current_map_p = gdb.parse_and_eval("address_space_memory.current_map")
+current_map = current_map_p.dereference()
+for cur in range(current_map["nr"]):
+flat_range = (current_map["ranges"] + cur).dereference()
+mr = flat_range["mr"].dereference()
+
+# we only care about RAM
+if (not mr["ram"]):
+continue
+
+section_size = int128_get64(flat_range["addr"]["size"])
+target_start = int128_get64(flat_range["addr"]["start"])
+target_end = target_start + section_size
+host_addr= (memory_region_get_ram_ptr(mr) +
+flat_range["offset_in_region"])
+predecessor = None
+
+# find continuity in guest physical address space
+if (len(guest_phys_blocks) > 0):
+predecessor = guest_phys_blocks[-1]
+predecessor_size = (predecessor["target_end"] -
+predecessor["target_start"])
+
+# the memory API guarantees monotonically increasing
+# traversal
+assert (predecessor["target_end"] <= target_start)
+
+# we want continuity in both guest-physical and
+# host-virtual memory
+if (predecessor["target_end"] < target_start or
+predecessor["host_addr"] + predecessor_size != host_addr):
+predecessor = None
+
+if (predecessor is None):
+# isolated mapping, add it to the list
+guest_phys_blocks.append({"target_start": target_start,
+ "target_end" : target_end,
+ "host_addr" : host_addr})
+message = "added"
+else:
+# expand predecessor until @target_end; predecessor's
+# start doesn't change
+predecessor["target_end"] = target_end
+message = "joined"
+
+print ("%016x %016x %016x %-7s %5u" %
+ (target_start, target_end, host_addr.cast(UINTPTR_T),
+message, len(guest_phys_blocks)))
+
+return guest_phys_blocks
+
+
class DumpGuestMemory(gdb.Command):
"""Extract guest vmcore from qemu process coredump.
@@ -100,96 +190,9 @@ shape and this command should mostly work."""
super(DumpGuestMemory, self).__init__("dump-guest-memory",
gdb.COMMAND_DATA,
gdb.COMPLETE_FILENAME)
-self.uintptr_t = gdb.lookup_type("uintptr_t")
self.elf64_ehdr_le = struct.Struct("<%s" % ELF64_EHDR)
self.elf64_phdr_le = struct.Struct("<%s" % ELF64_PHDR)
-
-def int128_get64(self, val):
-assert (val["hi"] == 0)
-return val["lo"]
-
-def qlist_foreach(self, head, field_str):
-var_p = head["lh_first"]
-while (var_p != 0):
-var = var_p.dereference()
-
[Qemu-devel] [PATCH v4] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties
This patch adds support for qom-type-prop-list command to list object
class properties. A later patch will use this functionality to
implement x86_64-cpu properties.
Signed-off-by: Valentin Rakush
Cc: Luiz Capitulino
Cc: Eric Blake
Cc: Markus Armbruster
Cc: Andreas Färber
Cc: Daniel P. Berrange
---
V4: review fixes
- the typename argument in the hmp command changed to be mandatory
V3: commit message fix
- commit message changed to reflect actual command name
V2: Fixes after first review
- changed command name from qom-type-list to qom-type-prop-list
- changed memory allocation from g_malloc0 to g_new0
- changed parameter name from path to typename
- fixed wordings and comments
- fixed source code formatting
- registered the command in monitor
hmp-commands.hx | 13 +
hmp.c| 21 +
hmp.h| 1 +
include/qom/object.h | 31 +++
qapi-schema.json | 19 +++
qmp-commands.hx | 6 ++
qmp.c| 32
qom/object.c | 7 +++
8 files changed, 130 insertions(+)
diff --git a/hmp-commands.hx b/hmp-commands.hx
index bb52e4d..ee4d1e2 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1734,6 +1734,19 @@ Print QOM properties of object at location @var{path}
ETEXI
{
+.name = "qom-type-prop-list",
+.args_type = "typename:s",
+.params = "typename",
+.help = "list QOM class properties",
+.mhandler.cmd = hmp_qom_type_prop_list,
+},
+
+STEXI
+@item qom-type-prop-list [@var{typename}]
+Print QOM properties of the type @var{typename}
+ETEXI
+
+{
.name = "qom-set",
.args_type = "path:s,property:s,value:s",
.params = "path property value",
diff --git a/hmp.c b/hmp.c
index 54f2620..6de9bf0 100644
--- a/hmp.c
+++ b/hmp.c
@@ -2052,6 +2052,27 @@ void hmp_qom_list(Monitor *mon, const QDict *qdict)
hmp_handle_error(mon, &err);
}
+void hmp_qom_type_prop_list(Monitor *mon, const QDict *qdict)
+{
+const char *typename = qdict_get_try_str(qdict, "typename");
+ObjectPropertyInfoList *list;
+Error *err = NULL;
+
+list = qmp_qom_type_prop_list(typename, &err);
+if (!err) {
+ObjectPropertyInfoList *start = list;
+while (list) {
+ObjectPropertyInfo *value = list->value;
+
+monitor_printf(mon, "%s (%s)\n",
+ value->name, value->type);
+list = list->next;
+}
+qapi_free_ObjectPropertyInfoList(start);
+}
+hmp_handle_error(mon, &err);
+}
+
void hmp_qom_set(Monitor *mon, const QDict *qdict)
{
const char *path = qdict_get_str(qdict, "path");
diff --git a/hmp.h b/hmp.h
index a8c5b5a..8c12ebe 100644
--- a/hmp.h
+++ b/hmp.h
@@ -103,6 +103,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict);
void hmp_info_memdev(Monitor *mon, const QDict *qdict);
void hmp_info_memory_devices(Monitor *mon, const QDict *qdict);
void hmp_qom_list(Monitor *mon, const QDict *qdict);
+void hmp_qom_type_prop_list(Monitor *mon, const QDict *qdict);
void hmp_qom_set(Monitor *mon, const QDict *qdict);
void object_add_completion(ReadLineState *rs, int nb_args, const char *str);
void object_del_completion(ReadLineState *rs, int nb_args, const char *str);
diff --git a/include/qom/object.h b/include/qom/object.h
index d0dafe9..0c8379d 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1013,6 +1013,37 @@ void object_property_iter_init(ObjectPropertyIterator
*iter,
*/
ObjectProperty *object_property_iter_next(ObjectPropertyIterator *iter);
+/**
+ * object_class_property_iter_init:
+ * @klass: the class owning the properties to be iterated over
+ *
+ * Initializes an iterator for traversing all properties
+ * registered against a class type and all parent classes.
+ *
+ * It is forbidden to modify the property list while iterating,
+ * whether removing or adding properties.
+ *
+ * NB For getting next property in the list the object related
+ * function object_property_iter_next is still used.
+ *
+ * Typical usage pattern would be
+ *
+ *
+ * Using object class property iterators
+ *
+ * ObjectProperty *prop;
+ * ObjectPropertyIterator iter;
+ *
+ * object_class property_iter_init(&iter, obj);
+ * while ((prop = object_property_iter_next(&iter))) {
+ * ... do something with prop ...
+ * }
+ *
+ *
+ */
+void object_class_property_iter_init(ObjectPropertyIterator *iter,
+ ObjectClass *klass);
+
void object_unparent(Object *obj);
/**
diff --git a/qapi-schema.json b/qapi-schema.json
index b3038b2..2e960db 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4081,3 +4081,22 @@
##
{ 'enum': 'ReplayMode',
'data': [ 'none', 'record', 'play' ] }
+
+##
+# @qom-type-prop-list:
+#
+# This command will list any properties of an object class
+# given its typena
[Qemu-devel] [PATCH 03/10] s390x/sclp: add device to the sysbus in sclp_realize
From: David Hildenbrand
The init of a device should have no side effects. Therefore move
registering of the event facility into the realize function, so
multiple instances of the SCLP device can be created e.g. for
introspection.
Add some more detail as to why we have to add it to the sysbus
at all.
Suggested-by: Paolo Bonzini
Signed-off-by: David Hildenbrand
Signed-off-by: Cornelia Huck
---
hw/s390x/sclp.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 74f2b40..23dee88 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -465,6 +465,12 @@ static void sclp_realize(DeviceState *dev, Error **errp)
if (err) {
goto out;
}
+/*
+ * qdev_device_add searches the sysbus for TYPE_SCLP_EVENTS_BUS. As long
+ * as we can't find a fitting bus via the qom tree, we have to add the
+ * event facility to the sysbus, so e.g. a sclp console can be created.
+ */
+qdev_set_parent_bus(DEVICE(sclp->event_facility), sysbus_get_default());
ret = s390_set_memory_limit(machine->maxram_size, &hw_limit);
if (ret == -E2BIG) {
@@ -533,8 +539,6 @@ static void sclp_init(Object *obj)
new = object_new(TYPE_SCLP_EVENT_FACILITY);
object_property_add_child(obj, TYPE_SCLP_EVENT_FACILITY, new, NULL);
-/* qdev_device_add searches the sysbus for TYPE_SCLP_EVENTS_BUS */
-qdev_set_parent_bus(DEVICE(new), sysbus_get_default());
object_unref(new);
sclp->event_facility = EVENT_FACILITY(new);
--
2.7.0
[Qemu-devel] [PATCH 01/10] s390x/skeys: Fix instance and class size
From: Christian Borntraeger
fix a typo that messes up instance and class size.
Signed-off-by: Christian Borntraeger
Reviewed-by: David Hildenbrand
Signed-off-by: Cornelia Huck
---
hw/s390x/s390-skeys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index f2b732e..0cd3407 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -237,7 +237,7 @@ static const TypeInfo qemu_s390_skeys_info = {
.instance_init = qemu_s390_skeys_init,
.instance_size = sizeof(QEMUS390SKeysState),
.class_init= qemu_s390_skeys_class_init,
-.instance_size = sizeof(S390SKeysClass),
+.class_size= sizeof(S390SKeysClass),
};
static void s390_storage_keys_save(QEMUFile *f, void *opaque)
--
2.7.0
[Qemu-devel] [PATCH 00/10] Next round of s390x patches
Hi, here's the next round of s390x patches. Mostly bugfixes and small improvements; and the gdb target.xml patch. Want to send a pullreq next week. Bo Tu (2): watchdog: introduction of get_watchdog_action watchdog/diag288: don't reset for action=none|debug|pause Christian Borntraeger (2): s390x/skeys: Fix instance and class size s390x/machine: make addon register fields static David Hildenbrand (3): s390x/sclp: add device to the sysbus in sclp_realize gdb: provide the name of the architecture in the target.xml s390x: s390_cpu_get_phys_page_debug has to return -1 Halil Pasic (1): s390x/css: fix control flags during csch Pierre Morel (1): s390x/ioinst: set type and len for SEI response Song Shan Gong (1): s390x: fix generation of event information crw gdbstub.c | 21 ++--- hw/s390x/css.c | 15 +-- hw/s390x/css.h | 1 + hw/s390x/s390-skeys.c | 2 +- hw/s390x/sclp.c | 8 ++-- hw/watchdog/watchdog.c | 14 +- hw/watchdog/wdt_diag288.c | 7 +++ include/qom/cpu.h | 3 +++ include/sysemu/watchdog.h | 10 ++ target-arm/cpu.c| 12 target-arm/cpu64.c | 6 ++ target-ppc/translate_init.c | 10 ++ target-s390x/cpu.c | 6 ++ target-s390x/helper.c | 5 +++-- target-s390x/ioinst.c | 4 +++- target-s390x/machine.c | 4 ++-- 16 files changed, 102 insertions(+), 26 deletions(-) -- 2.7.0
[Qemu-devel] [PATCH 06/10] watchdog: introduction of get_watchdog_action
From: Bo Tu
Add get_watchdog_action(void) to allow access to the configured action.
Reviewed-by: David Hildenbrand
Signed-off-by: Bo Tu
Signed-off-by: Cornelia Huck
---
hw/watchdog/watchdog.c| 14 +-
include/sysemu/watchdog.h | 10 ++
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index 8d4b0ee..81de0e5 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -29,15 +29,6 @@
#include "qapi-event.h"
#include "hw/nmi.h"
-/* Possible values for action parameter. */
-#define WDT_RESET1 /* Hard reset. */
-#define WDT_SHUTDOWN 2 /* Shutdown. */
-#define WDT_POWEROFF 3 /* Quit. */
-#define WDT_PAUSE4 /* Pause. */
-#define WDT_DEBUG5 /* Prints a message and continues running. */
-#define WDT_NONE 6 /* Do nothing. */
-#define WDT_NMI 7 /* Inject nmi into the guest */
-
static int watchdog_action = WDT_RESET;
static QLIST_HEAD(watchdog_list, WatchdogTimerModel) watchdog_list;
@@ -105,6 +96,11 @@ int select_watchdog_action(const char *p)
return 0;
}
+int get_watchdog_action(void)
+{
+return watchdog_action;
+}
+
/* This actually performs the "action" once a watchdog has expired,
* ie. reboot, shutdown, exit, etc.
*/
diff --git a/include/sysemu/watchdog.h b/include/sysemu/watchdog.h
index 3e9a970..72a4da0 100644
--- a/include/sysemu/watchdog.h
+++ b/include/sysemu/watchdog.h
@@ -24,6 +24,15 @@
#include "qemu/queue.h"
+/* Possible values for action parameter. */
+#define WDT_RESET1 /* Hard reset. */
+#define WDT_SHUTDOWN 2 /* Shutdown. */
+#define WDT_POWEROFF 3 /* Quit. */
+#define WDT_PAUSE4 /* Pause. */
+#define WDT_DEBUG5 /* Prints a message and continues running. */
+#define WDT_NONE 6 /* Do nothing. */
+#define WDT_NMI 7 /* Inject nmi into the guest. */
+
struct WatchdogTimerModel {
QLIST_ENTRY(WatchdogTimerModel) entry;
@@ -37,6 +46,7 @@ typedef struct WatchdogTimerModel WatchdogTimerModel;
/* in hw/watchdog.c */
int select_watchdog(const char *p);
int select_watchdog_action(const char *action);
+int get_watchdog_action(void);
void watchdog_add_model(WatchdogTimerModel *model);
void watchdog_perform_action(void);
--
2.7.0
[Qemu-devel] [PATCH 05/10] s390x: fix generation of event information crw
From: Song Shan Gong
Only one channel report word (crw) may be pending if there is
event-information pending.
This patch introduces a bool-type field 'sei_pending' for the
channel subsystem, which indicates whether there are pending events.
It is set when event information is made pending and the crw
generated, and cleared after the guest has collected all pending
event information. A crw is not generated if this flag had already
been set.
Signed-off-by: Song Shan Gong
Reviewed-by: Cornelia Huck
Signed-off-by: Cornelia Huck
---
hw/s390x/css.c| 13 -
hw/s390x/css.h| 1 +
target-s390x/ioinst.c | 1 +
3 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/css.c b/hw/s390x/css.c
index 343c352..533357a 100644
--- a/hw/s390x/css.c
+++ b/hw/s390x/css.c
@@ -49,6 +49,7 @@ typedef struct IoAdapter {
typedef struct ChannelSubSys {
QTAILQ_HEAD(, CrwContainer) pending_crws;
+bool sei_pending;
bool do_crw_mchk;
bool crws_lost;
uint8_t max_cssid;
@@ -1359,7 +1360,15 @@ void css_generate_chp_crws(uint8_t cssid, uint8_t chpid)
void css_generate_css_crws(uint8_t cssid)
{
-css_queue_crw(CRW_RSC_CSS, 0, 0, cssid);
+if (!channel_subsys->sei_pending) {
+css_queue_crw(CRW_RSC_CSS, 0, 0, cssid);
+}
+channel_subsys->sei_pending = true;
+}
+
+void css_clear_sei_pending(void)
+{
+channel_subsys->sei_pending = false;
}
int css_enable_mcsse(void)
@@ -1509,6 +1518,7 @@ static void css_init(void)
{
channel_subsys = g_malloc0(sizeof(*channel_subsys));
QTAILQ_INIT(&channel_subsys->pending_crws);
+channel_subsys->sei_pending = false;
channel_subsys->do_crw_mchk = true;
channel_subsys->crws_lost = false;
channel_subsys->chnmon_active = false;
@@ -1561,6 +1571,7 @@ void css_reset(void)
QTAILQ_REMOVE(&channel_subsys->pending_crws, crw_cont, sibling);
g_free(crw_cont);
}
+channel_subsys->sei_pending = false;
channel_subsys->do_crw_mchk = true;
channel_subsys->crws_lost = false;
diff --git a/hw/s390x/css.h b/hw/s390x/css.h
index a09bb1f..a47937d 100644
--- a/hw/s390x/css.h
+++ b/hw/s390x/css.h
@@ -103,6 +103,7 @@ void css_generate_sch_crws(uint8_t cssid, uint8_t ssid,
uint16_t schid,
int hotplugged, int add);
void css_generate_chp_crws(uint8_t cssid, uint8_t chpid);
void css_generate_css_crws(uint8_t cssid);
+void css_clear_sei_pending(void);
void css_adapter_interrupt(uint8_t isc);
#define CSS_IO_ADAPTER_VIRTIO 1
diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
index 57c2d8b..9a8de6d 100644
--- a/target-s390x/ioinst.c
+++ b/target-s390x/ioinst.c
@@ -614,6 +614,7 @@ static void ioinst_handle_chsc_sei(ChscReq *req, ChscResp
*res)
(*res_flags) |= 0x80;
} else {
(*res_flags) &= ~0x80;
+css_clear_sei_pending();
}
} else {
res->code = cpu_to_be16(0x0005);
--
2.7.0
[Qemu-devel] [PATCH 02/10] s390x/machine: make addon register fields static
From: Christian Borntraeger
No need to have them as global symbol.
Acked-by: Cornelia Huck
Signed-off-by: Christian Borntraeger
Signed-off-by: Cornelia Huck
---
target-s390x/machine.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target-s390x/machine.c b/target-s390x/machine.c
index b76fb08..96ad0a9 100644
--- a/target-s390x/machine.c
+++ b/target-s390x/machine.c
@@ -48,7 +48,7 @@ static inline bool fpu_needed(void *opaque)
return true;
}
-const VMStateDescription vmstate_fpu = {
+static const VMStateDescription vmstate_fpu = {
.name = "cpu/fpu",
.version_id = 1,
.minimum_version_id = 1,
@@ -75,7 +75,7 @@ const VMStateDescription vmstate_fpu = {
}
};
-const VMStateDescription vmstate_vregs = {
+static const VMStateDescription vmstate_vregs = {
.name = "cpu/vregs",
.version_id = 1,
.minimum_version_id = 1,
--
2.7.0
[Qemu-devel] [PATCH 04/10] s390x/ioinst: set type and len for SEI response
From: Pierre Morel
If no event information is pending, the return code
is set to 0x0005 and the length of the response is
set to 8 bytes.
Signed-off-by: Pierre Morel
Reviewed-by: Cornelia Huck
Reviewed-by: Song Shan Gong
Cc: qemu-sta...@nongnu.org
Signed-off-by: Cornelia Huck
---
target-s390x/ioinst.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target-s390x/ioinst.c b/target-s390x/ioinst.c
index 77f2a1f..57c2d8b 100644
--- a/target-s390x/ioinst.c
+++ b/target-s390x/ioinst.c
@@ -616,7 +616,8 @@ static void ioinst_handle_chsc_sei(ChscReq *req, ChscResp
*res)
(*res_flags) &= ~0x80;
}
} else {
-res->code = cpu_to_be16(0x0004);
+res->code = cpu_to_be16(0x0005);
+res->len = cpu_to_be16(CHSC_MIN_RESP_LEN);
}
}
--
2.7.0
[Qemu-devel] [PATCH v2 0/6] virtio-gpu: fixes and spice preparation
Hi, This patch series brings some minor opengl bug fixes and also some virtio-gpu preparations for spice support. v2: address review comments from Marc-André Lureau (mostly patch #4) Gerd Hoffmann (6): zap qemu_egl_has_ext in include/ui/egl-helpers.h console: block rendering until client is done virtio-gpu: fix memory leak in error path virtio-gpu: maintain command queue virtio-gpu: add support to enable/disable command processing virtio-gpu: block any rendering until client (ui) is done hw/display/virtio-gpu-3d.c | 11 -- hw/display/virtio-gpu.c| 77 ++ hw/display/virtio-vga.c| 10 ++ include/hw/virtio/virtio-gpu.h | 4 +++ include/ui/console.h | 2 ++ include/ui/egl-helpers.h | 1 - ui/console.c | 10 ++ 7 files changed, 90 insertions(+), 25 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH v2 1/6] zap qemu_egl_has_ext in include/ui/egl-helpers.h
Drop leftover prototype which sneaked in by mistake Signed-off-by: Gerd Hoffmann Reviewed-by: Marc-André Lureau --- include/ui/egl-helpers.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/ui/egl-helpers.h b/include/ui/egl-helpers.h index 5ad5dc3..8c84398 100644 --- a/include/ui/egl-helpers.h +++ b/include/ui/egl-helpers.h @@ -11,6 +11,5 @@ EGLSurface qemu_egl_init_surface_x11(EGLContext ectx, Window win); int qemu_egl_init_dpy(EGLNativeDisplayType dpy, bool gles, bool debug); EGLContext qemu_egl_init_ctx(void); -bool qemu_egl_has_ext(const char *haystack, const char *needle); #endif /* EGL_HELPERS_H */ -- 1.8.3.1
[Qemu-devel] [PATCH 09/10] gdb: provide the name of the architecture in the target.xml
From: David Hildenbrand
This patch provides the name of the architecture in the target.xml
if available.
This allows the remote gdb to detect the target architecture on its
own - so there is no need to specify it manually (e.g. if gdb is
started without a binary) using "set arch *arch_name*".
The name of the architecture is provided by a callback that can
be implemented by all architectures. The arm implementation has
special handling for iwmmxt and returns arm otherwise. This can
be extended if necessary.
Signed-off-by: David Hildenbrand
Acked-by: Cornelia Huck
Signed-off-by: Christian Borntraeger
[rework to use a callback]
Message-Id: <1449144881-130935-1-git-send-email-borntrae...@de.ibm.com>
Reviewed-by: Peter Maydell
Signed-off-by: Cornelia Huck
---
gdbstub.c | 21 ++---
include/qom/cpu.h | 3 +++
target-arm/cpu.c| 12
target-arm/cpu64.c | 6 ++
target-ppc/translate_init.c | 10 ++
target-s390x/cpu.c | 6 ++
6 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index 1a84c1a..59d1650 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -540,13 +540,20 @@ static const char *get_feature_xml(const char *p, const
char **newp,
GDBRegisterState *r;
CPUState *cpu = first_cpu;
-snprintf(target_xml, sizeof(target_xml),
- ""
- ""
- ""
- "",
- cc->gdb_core_xml_file);
-
+pstrcat(target_xml, sizeof(target_xml),
+""
+""
+"");
+if (cc->gdb_arch_name) {
+gchar *arch = cc->gdb_arch_name(cpu);
+pstrcat(target_xml, sizeof(target_xml), "");
+pstrcat(target_xml, sizeof(target_xml), arch);
+pstrcat(target_xml, sizeof(target_xml), "");
+g_free(arch);
+}
+pstrcat(target_xml, sizeof(target_xml), "gdb_core_xml_file);
+pstrcat(target_xml, sizeof(target_xml), "\"/>");
for (r = cpu->gdb_regs; r; r = r->next) {
pstrcat(target_xml, sizeof(target_xml), "xml);
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 2e5229d..035179c 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -120,6 +120,8 @@ struct TranslationBlock;
* @gdb_core_xml_file: File name for core registers GDB XML description.
* @gdb_stop_before_watchpoint: Indicates whether GDB expects the CPU to stop
* before the insn which triggers a watchpoint rather than after it.
+ * @gdb_arch_name: Optional callback that returns the architecture name known
+ * to GDB. The caller must free the returned string with g_free.
* @cpu_exec_enter: Callback for cpu_exec preparation.
* @cpu_exec_exit: Callback for cpu_exec cleanup.
* @cpu_exec_interrupt: Callback for processing interrupts in cpu_exec.
@@ -177,6 +179,7 @@ typedef struct CPUClass {
const struct VMStateDescription *vmsd;
int gdb_num_core_regs;
const char *gdb_core_xml_file;
+gchar * (*gdb_arch_name)(CPUState *cpu);
bool gdb_stop_before_watchpoint;
void (*cpu_exec_enter)(CPUState *cpu);
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 6c34476..0e582c4 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -1426,6 +1426,17 @@ static int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr
address, int rw,
}
#endif
+static gchar *arm_gdb_arch_name(CPUState *cs)
+{
+ARMCPU *cpu = ARM_CPU(cs);
+CPUARMState *env = &cpu->env;
+
+if (arm_feature(env, ARM_FEATURE_IWMMXT)) {
+return g_strdup("iwmmxt");
+}
+return g_strdup("arm");
+}
+
static void arm_cpu_class_init(ObjectClass *oc, void *data)
{
ARMCPUClass *acc = ARM_CPU_CLASS(oc);
@@ -1460,6 +1471,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void
*data)
#endif
cc->gdb_num_core_regs = 26;
cc->gdb_core_xml_file = "arm-core.xml";
+cc->gdb_arch_name = arm_gdb_arch_name;
cc->gdb_stop_before_watchpoint = true;
cc->debug_excp_handler = arm_debug_excp_handler;
diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index cc177bb..c847513 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -287,6 +287,11 @@ static void aarch64_cpu_set_pc(CPUState *cs, vaddr value)
}
}
+static gchar *aarch64_gdb_arch_name(CPUState *cs)
+{
+return g_strdup("aarch64");
+}
+
static void aarch64_cpu_class_init(ObjectClass *oc, void *data)
{
CPUClass *cc = CPU_CLASS(oc);
@@ -297,6 +302,7 @@ static void aarch64_cpu_class_init(ObjectClass *oc, void
*data)
cc->gdb_write_register = aarch64_cpu_gdb_write_register;
cc->gdb_num_core_regs = 34;
cc->gdb_core_xml_file = "aarch64-core.xml";
+cc->gdb_arch_name = aarch64_gdb_arch_name;
}
static void aarch64_cpu_register(const ARMCPUInfo *info)
diff --git a/target-ppc/translate_init.c b/target-p
[Qemu-devel] [PATCH 10/10] s390x: s390_cpu_get_phys_page_debug has to return -1
From: David Hildenbrand
If translation fails, we have to return -1. For now, we
would simply return the value last stored to raddr (if any).
This way, reading invalid memory via gdb will return values, although it
shouldn't.
Reviewed-by: Christian Borntraeger
Signed-off-by: David Hildenbrand
Signed-off-by: Cornelia Huck
---
target-s390x/helper.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index e541d69..1231e58 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -162,8 +162,9 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr
vaddr)
vaddr &= 0x7fff;
}
-mmu_translate(env, vaddr, MMU_INST_FETCH, asc, &raddr, &prot, false);
-
+if (mmu_translate(env, vaddr, MMU_INST_FETCH, asc, &raddr, &prot, false)) {
+return -1;
+}
return raddr;
}
--
2.7.0
[Qemu-devel] [PATCH 08/10] s390x/css: fix control flags during csch
From: Halil Pasic >From the beginning, css support contained an error in csch handling: instead of setting the clear bit in the function control bits twice, we need to set the clear pending bit in the activity control bits. Let's fix this. Cc: qemu-sta...@nongnu.org Reviewed-by: Cornelia Huck Signed-off-by: Halil Pasic Signed-off-by: Cornelia Huck --- hw/s390x/css.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/s390x/css.c b/hw/s390x/css.c index 533357a..e83bf60 100644 --- a/hw/s390x/css.c +++ b/hw/s390x/css.c @@ -702,7 +702,7 @@ int css_do_csch(SubchDev *sch) /* Trigger the clear function. */ s->ctrl &= ~(SCSW_CTRL_MASK_FCTL | SCSW_CTRL_MASK_ACTL); -s->ctrl |= SCSW_FCTL_CLEAR_FUNC | SCSW_FCTL_CLEAR_FUNC; +s->ctrl |= SCSW_FCTL_CLEAR_FUNC | SCSW_ACTL_CLEAR_PEND; do_subchannel_work(sch, NULL); ret = 0; -- 2.7.0
[Qemu-devel] [PATCH v2 6/6] virtio-gpu: block any rendering until client (ui) is done
Wire up gl_block callback, so ui code can request to stop
virtio-gpu rendering.
Signed-off-by: Gerd Hoffmann
---
hw/display/virtio-gpu-3d.c | 5 +
hw/display/virtio-gpu.c| 11 +++
hw/display/virtio-vga.c| 10 ++
include/hw/virtio/virtio-gpu.h | 1 +
4 files changed, 27 insertions(+)
diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index d6487f7..8906316 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -382,6 +382,11 @@ void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
{
VIRTIO_GPU_FILL_CMD(cmd->cmd_hdr);
+cmd->waiting = g->renderer_blocked;
+if (cmd->waiting) {
+return;
+}
+
virgl_renderer_force_ctx_0();
switch (cmd->cmd_hdr.type) {
case VIRTIO_GPU_CMD_CTX_CREATE:
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index e341d6e..ce02194 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -896,11 +896,22 @@ static int virtio_gpu_ui_info(void *opaque, uint32_t idx,
QemuUIInfo *info)
return 0;
}
+static void virtio_gpu_gl_block(void *opaque, bool block)
+{
+VirtIOGPU *g = opaque;
+
+g->renderer_blocked = block;
+if (!block) {
+virtio_gpu_process_cmdq(g);
+}
+}
+
const GraphicHwOps virtio_gpu_ops = {
.invalidate = virtio_gpu_invalidate_display,
.gfx_update = virtio_gpu_update_display,
.text_update = virtio_gpu_text_update,
.ui_info = virtio_gpu_ui_info,
+.gl_block = virtio_gpu_gl_block,
};
static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp)
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index f7e539f..255d419 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -65,11 +65,21 @@ static int virtio_vga_ui_info(void *opaque, uint32_t idx,
QemuUIInfo *info)
return -1;
}
+static void virtio_vga_gl_block(void *opaque, bool block)
+{
+VirtIOVGA *vvga = opaque;
+
+if (virtio_gpu_ops.gl_block) {
+virtio_gpu_ops.gl_block(&vvga->vdev, block);
+}
+}
+
static const GraphicHwOps virtio_vga_ops = {
.invalidate = virtio_vga_invalidate_display,
.gfx_update = virtio_vga_update_display,
.text_update = virtio_vga_text_update,
.ui_info = virtio_vga_ui_info,
+.gl_block = virtio_vga_gl_block,
};
/* VGA device wrapper around PCI device around virtio GPU */
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index f6cae0b..13b0ab0 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -107,6 +107,7 @@ typedef struct VirtIOGPU {
bool use_virgl_renderer;
bool renderer_inited;
+bool renderer_blocked;
QEMUTimer *fence_poll;
QEMUTimer *print_stats;
--
1.8.3.1
[Qemu-devel] [PATCH v2 3/6] virtio-gpu: fix memory leak in error path
Found by Coverity Scan, buf not freed on error.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Marc-André Lureau
---
hw/display/virtio-gpu-3d.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index 28dccfd..d2af327 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -197,7 +197,7 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
qemu_log_mask(LOG_GUEST_ERROR, "%s: size mismatch (%zd/%d)",
__func__, s, cs.size);
cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
-return;
+goto out;
}
if (virtio_gpu_stats_enabled(g->conf)) {
@@ -207,6 +207,7 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
virgl_renderer_submit_cmd(buf, cs.hdr.ctx_id, cs.size / 4);
+out:
g_free(buf);
}
--
1.8.3.1
[Qemu-devel] [PATCH 07/10] watchdog/diag288: don't reset for action=none|debug|pause
From: Bo Tu
If the watchdog expires and the guest is not notified (NONE, DEBUG, PAUSE),
we must not reset the watchdog device, otherwise watchdog_ping() and
watchdog_stop() will fail when triggered by the guest. This reset behavior
matches to the z/VM behavior when a custom command is to be executed
on expiry.
Reviewed-by: David Hildenbrand
Signed-off-by: Bo Tu
Signed-off-by: Cornelia Huck
---
hw/watchdog/wdt_diag288.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/hw/watchdog/wdt_diag288.c b/hw/watchdog/wdt_diag288.c
index 2a885a4..ddda723 100644
--- a/hw/watchdog/wdt_diag288.c
+++ b/hw/watchdog/wdt_diag288.c
@@ -51,6 +51,13 @@ static void diag288_timer_expired(void *dev)
{
qemu_log_mask(CPU_LOG_RESET, "Watchdog timer expired.\n");
watchdog_perform_action();
+/* Reset the watchdog only if the guest was notified about expiry. */
+switch (get_watchdog_action()) {
+case WDT_DEBUG:
+case WDT_NONE:
+case WDT_PAUSE:
+ return;
+}
wdt_diag288_reset(dev);
}
--
2.7.0
[Qemu-devel] [PATCH v2 5/6] virtio-gpu: add support to enable/disable command processing
So we can stop rendering for a while in case we have to.
Signed-off-by: Gerd Hoffmann
Reviewed-by: Marc-André Lureau
---
hw/display/virtio-gpu-3d.c | 3 ++-
hw/display/virtio-gpu.c| 5 -
include/hw/virtio/virtio-gpu.h | 2 ++
3 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index d2af327..d6487f7 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -553,7 +553,8 @@ static void virtio_gpu_fence_poll(void *opaque)
VirtIOGPU *g = opaque;
virgl_renderer_poll();
-if (g->inflight) {
+virtio_gpu_process_cmdq(g);
+if (!QTAILQ_EMPTY(&g->cmdq) || !QTAILQ_EMPTY(&g->fenceq)) {
timer_mod(g->fence_poll, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + 10);
}
}
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index d4ee34d..e341d6e 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -754,7 +754,7 @@ static void virtio_gpu_handle_cursor_cb(VirtIODevice *vdev,
VirtQueue *vq)
qemu_bh_schedule(g->cursor_bh);
}
-static void virtio_gpu_process_cmdq(VirtIOGPU *g)
+void virtio_gpu_process_cmdq(VirtIOGPU *g)
{
struct virtio_gpu_ctrl_command *cmd;
@@ -764,6 +764,9 @@ static void virtio_gpu_process_cmdq(VirtIOGPU *g)
/* process command */
VIRGL(g, virtio_gpu_virgl_process_cmd, virtio_gpu_simple_process_cmd,
g, cmd);
+if (cmd->waiting) {
+break;
+}
QTAILQ_REMOVE(&g->cmdq, cmd, next);
if (virtio_gpu_stats_enabled(g->conf)) {
g->stats.requests++;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index f7e7a52..f6cae0b 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -76,6 +76,7 @@ struct virtio_gpu_ctrl_command {
VirtQueue *vq;
struct virtio_gpu_ctrl_hdr cmd_hdr;
uint32_t error;
+bool waiting;
bool finished;
QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
};
@@ -152,6 +153,7 @@ int virtio_gpu_create_mapping_iov(struct
virtio_gpu_resource_attach_backing *ab,
struct virtio_gpu_ctrl_command *cmd,
struct iovec **iov);
void virtio_gpu_cleanup_mapping_iov(struct iovec *iov, uint32_t count);
+void virtio_gpu_process_cmdq(VirtIOGPU *g);
/* virtio-gpu-3d.c */
void virtio_gpu_virgl_process_cmd(VirtIOGPU *g,
--
1.8.3.1
[Qemu-devel] [PATCH v2 2/6] console: block rendering until client is done
Allow gl user interfaces to block display device gl rendering.
The ui code might want to do that in case it takes a little
longer to bring things to screen, for example because we'll
hand over a dma-buf to another process (spice will do that).
Signed-off-by: Gerd Hoffmann
Reviewed-by: Marc-André Lureau
---
include/ui/console.h | 2 ++
ui/console.c | 10 ++
2 files changed, 12 insertions(+)
diff --git a/include/ui/console.h b/include/ui/console.h
index adac36d..12ad627 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -362,6 +362,7 @@ typedef struct GraphicHwOps {
void (*text_update)(void *opaque, console_ch_t *text);
void (*update_interval)(void *opaque, uint64_t interval);
int (*ui_info)(void *opaque, uint32_t head, QemuUIInfo *info);
+void (*gl_block)(void *opaque, bool block);
} GraphicHwOps;
QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head,
@@ -374,6 +375,7 @@ void graphic_console_set_hwops(QemuConsole *con,
void graphic_hw_update(QemuConsole *con);
void graphic_hw_invalidate(QemuConsole *con);
void graphic_hw_text_update(QemuConsole *con, console_ch_t *chardata);
+void graphic_hw_gl_block(QemuConsole *con, bool block);
QemuConsole *qemu_console_lookup_by_index(unsigned int index);
QemuConsole *qemu_console_lookup_by_device(DeviceState *dev, uint32_t head);
diff --git a/ui/console.c b/ui/console.c
index fe950c6..791b4fc 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -261,6 +261,16 @@ void graphic_hw_update(QemuConsole *con)
}
}
+void graphic_hw_gl_block(QemuConsole *con, bool block)
+{
+if (!con) {
+con = active_console;
+}
+if (con && con->hw_ops->gl_block) {
+con->hw_ops->gl_block(con->hw, block);
+}
+}
+
void graphic_hw_invalidate(QemuConsole *con)
{
if (!con) {
--
1.8.3.1
[Qemu-devel] [PATCH v2 4/6] virtio-gpu: maintain command queue
We'll go take out the commands we receive out of the virt queue and put
them into a linked list, to decouple virtio queue handling from actual
command processing.
Also move cmd processing to new virtio_gpu_handle_ctrl func, so we can
easily kick it from different places.
Signed-off-by: Gerd Hoffmann
---
hw/display/virtio-gpu.c| 63 +++---
include/hw/virtio/virtio-gpu.h | 1 +
2 files changed, 42 insertions(+), 22 deletions(-)
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 7e79a9c..d4ee34d 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -754,33 +754,21 @@ static void virtio_gpu_handle_cursor_cb(VirtIODevice
*vdev, VirtQueue *vq)
qemu_bh_schedule(g->cursor_bh);
}
-static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+static void virtio_gpu_process_cmdq(VirtIOGPU *g)
{
-VirtIOGPU *g = VIRTIO_GPU(vdev);
struct virtio_gpu_ctrl_command *cmd;
-if (!virtio_queue_ready(vq)) {
-return;
-}
-
-#ifdef CONFIG_VIRGL
-if (!g->renderer_inited && g->use_virgl_renderer) {
-virtio_gpu_virgl_init(g);
-g->renderer_inited = true;
-}
-#endif
-
-cmd = g_new(struct virtio_gpu_ctrl_command, 1);
-while (virtqueue_pop(vq, &cmd->elem)) {
-cmd->vq = vq;
-cmd->error = 0;
-cmd->finished = false;
-if (virtio_gpu_stats_enabled(g->conf)) {
-g->stats.requests++;
-}
+while (!QTAILQ_EMPTY(&g->cmdq)) {
+cmd = QTAILQ_FIRST(&g->cmdq);
+/* process command */
VIRGL(g, virtio_gpu_virgl_process_cmd, virtio_gpu_simple_process_cmd,
g, cmd);
+QTAILQ_REMOVE(&g->cmdq, cmd, next);
+if (virtio_gpu_stats_enabled(g->conf)) {
+g->stats.requests++;
+}
+
if (!cmd->finished) {
QTAILQ_INSERT_TAIL(&g->fenceq, cmd, next);
g->inflight++;
@@ -790,11 +778,41 @@ static void virtio_gpu_handle_ctrl(VirtIODevice *vdev,
VirtQueue *vq)
}
fprintf(stderr, "inflight: %3d (+)\r", g->inflight);
}
-cmd = g_new(struct virtio_gpu_ctrl_command, 1);
+} else {
+g_free(cmd);
}
}
+}
+
+static void virtio_gpu_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq)
+{
+VirtIOGPU *g = VIRTIO_GPU(vdev);
+struct virtio_gpu_ctrl_command *cmd;
+
+if (!virtio_queue_ready(vq)) {
+return;
+}
+
+#ifdef CONFIG_VIRGL
+if (!g->renderer_inited && g->use_virgl_renderer) {
+virtio_gpu_virgl_init(g);
+g->renderer_inited = true;
+}
+#endif
+
+cmd = g_new(struct virtio_gpu_ctrl_command, 1);
+while (virtqueue_pop(vq, &cmd->elem)) {
+cmd->vq = vq;
+cmd->error = 0;
+cmd->finished = false;
+cmd->waiting = false;
+QTAILQ_INSERT_TAIL(&g->cmdq, cmd, next);
+cmd = g_new(struct virtio_gpu_ctrl_command, 1);
+}
g_free(cmd);
+virtio_gpu_process_cmdq(g);
+
#ifdef CONFIG_VIRGL
if (g->use_virgl_renderer) {
virtio_gpu_virgl_fence_poll(g);
@@ -920,6 +938,7 @@ static void virtio_gpu_device_realize(DeviceState *qdev,
Error **errp)
g->ctrl_bh = qemu_bh_new(virtio_gpu_ctrl_bh, g);
g->cursor_bh = qemu_bh_new(virtio_gpu_cursor_bh, g);
QTAILQ_INIT(&g->reslist);
+QTAILQ_INIT(&g->cmdq);
QTAILQ_INIT(&g->fenceq);
g->enabled_output_bitmask = 1;
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 9b279d7..f7e7a52 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -94,6 +94,7 @@ typedef struct VirtIOGPU {
DeviceState *qdev;
QTAILQ_HEAD(, virtio_gpu_simple_resource) reslist;
+QTAILQ_HEAD(, virtio_gpu_ctrl_command) cmdq;
QTAILQ_HEAD(, virtio_gpu_ctrl_command) fenceq;
struct virtio_gpu_scanout scanout[VIRTIO_GPU_MAX_SCANOUT];
--
1.8.3.1
Re: [Qemu-devel] [PATCH 0/6] scripts/dump-guest-memory.py: Add multi-arch support
On 22/01/2016 13:08, Janosch Frank wrote: > The dump guest memory script for extracting a Linux core from a qemu > core is currently limited to amd64 and python 2. > > With this series we add support for python 3 (while maintaining python > 2 support) and add the possibility to extract dumps from VMs with the > most common architectures. > > Tested on X86_64, s390 and Aarch64-le (thanks to Andrew Jones). > > RFC to PATCH: > Tested on X86_64 and Aarch64-le. > Fix wrongly indented return in get_guest_phys_blocks(). > Fix wrong indent for predecessor_size that happened when copying. > Fix inconsistent line wrapping. > Fix python 3 compatibility comment being added in the wrong commit. > Fix docstrings that get displayed by gdb. > Replaced ceil(len_desc / 4.0) with ((len_desc + 3) // 4) > Added patch that fixes module docstring to account for moved gdb > help string. > > Janosch Frank (6): > scripts/dump-guest-memory.py: Move constants to the top > scripts/dump-guest-memory.py: Make methods functions > scripts/dump-guest-memory.py: Improve python 3 compatibility > scripts/dump-guest-memory.py: Cleanup functions > scripts/dump-guest-memory.py: Introduce multi-arch support > scripts/dump-guest-memory.py: Fix module docstring > > scripts/dump-guest-memory.py | 754 > +++ > 1 file changed, 472 insertions(+), 282 deletions(-) > Queued, thanks. Paolo
Re: [Qemu-devel] [PATCH v2 3/3] target-arm: Implement the S2 MMU inputsize > pamax check
On Fri, Jan 22, 2016 at 11:45:57AM +, Alex Bennée wrote:
>
> Edgar E. Iglesias writes:
>
> > On Fri, Jan 22, 2016 at 10:28:43AM +, Alex Bennée wrote:
> >>
> >> Edgar E. Iglesias writes:
> >>
> >> > From: "Edgar E. Iglesias"
> >> >
> >> > Implement the inputsize > pamax check for Stage 2 translations.
> >> > We have multiple choices for how to respond to errors and
> >> > choose to fault.
> >> >
> >> > Signed-off-by: Edgar E. Iglesias
> >> > ---
> >> > target-arm/helper.c | 16
> >> > 1 file changed, 12 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/target-arm/helper.c b/target-arm/helper.c
> >> > index 4abeb4d..9a7ff5e 100644
> >> > --- a/target-arm/helper.c
> >> > +++ b/target-arm/helper.c
> >> > @@ -6808,7 +6808,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> >> > target_ulong address,
> >> > */
> >> > int startlevel = extract32(tcr->raw_tcr, 6, 2);
> >> > unsigned int pamax = arm_pamax(cpu);
> >> > -bool ok;
> >> > +bool ok = true;
> >> >
> >> > if (va_size == 32 || stride == 9) {
> >> > /* AArch32 or 4KB pages */
> >> > @@ -6818,9 +6818,17 @@ static bool get_phys_addr_lpae(CPUARMState *env,
> >> > target_ulong address,
> >> > level = 3 - startlevel;
> >> > }
> >> >
> >> > -/* Check that the starting level is valid. */
> >> > -ok = check_s2_startlevel(cpu, va_size == 64, level,
> >> > - inputsize, stride, pamax);
> >> > +if (va_size == 64 &&
> >> > +inputsize > pamax &&
> >> > +(arm_el_is_aa64(env, 1) || inputsize > 40)) {
> >>
> >> If va_size == 64 doesn't that imply arm_el_is_aa64(env, 1)? Looking
> >> further up the function it seems that is what sets va_size in the first
> >> place. I think that makes the inputsize > 40 check redundant.
> >
> > va_size == 64 is true if the EL corresponding to the translation _regime_
> > is in 64 bit mode (in this case EL2).
> >
> > EL1 may very well be in 32bit mode.
>
> Ahh yes, I missed that on the first reading. I think it might be clearer
> when reading the code to have the:
>
> bool is_aarch64_regime = (va_size == 64);
>
> And use that to make it clear. And then comment on later check that it's
> incompatible with EL1 being aarch32.
>
> >
> >>
> >> > +/* We have multiple choices but choose to fault. */
> >> > +ok = false;
> >> > +}
> >> > +if (ok) {
> >> > +/* Check that the starting level is valid. */
> >> > +ok = check_s2_startlevel(cpu, va_size == 64, level,
> >> > + inputsize, stride, pamax);
> >> > +}
> >> > if (!ok) {
> >> > /* AArch64 reports these as level 0 faults.
> >> > * AArch32 reports these as level 1 faults.
> >>
> >> I'm not a fan of the ok = true / ok = false / ok =
> >> check_s2_start_level() / if (!ok) ping-pong here as it is hard to
> >> follow. I'm not sure how you could make it cleaner to follow though.
> >> Maybe something like:
> >>
> >> /* For stage 2 translations the starting level is specified by the
> >> * VTCR_EL2.SL0 field (whose interpretation depends on the page
> >> size)
> >> */
> >> int startlevel = extract32(tcr->raw_tcr, 6, 2);
> >> unsigned int pamax = arm_pamax(cpu);
> >> bool is_aarch64_regime = (va_size == 64);
> >> bool ok;
> >>
> >> if (va_size == 32 || stride == 9) {
> >> /* AArch32 or 4KB pages */
> >> level = 2 - startlevel;
> >> } else {
> >> /* 16KB or 64KB pages */
> >> level = 3 - startlevel;
> >> }
> >>
> >> if (is_aarch64_regime &&
> >> inputsize > pamax) {
> >> /* We have multiple choices but choose to fault. */
> >> ok = false;
> >> } else {
> >> /* Check that the starting level is valid. */
> >> ok = check_s2_startlevel(cpu, is_aarch64_regime, level,
> >> inputsize, stride, pamax);
> >> }
> >> if (!ok) {
> >> /* AArch64 reports these as level 0 faults.
> >> * AArch32 reports these as level 1 faults.
> >> */
> >> level = is_aarch64_regime ? 0 : 1;
> >> fault_type = translation_fault;
> >> goto do_fault;
> >> }
> >>
> >> But I'm wondering if it just makes more sense to push the:
> >>
> >> is_aarch64_regime && inputsize > pamax
> >>
> >> Check into check_s2_startlevel? Then you could just have a simple call
> >> which succeeds or falls through to a fault?
> >
> > Yeah, I guess we could rename check_s2_startlevel to something more generic
> > and move all the checks there. I don't feel very strongly about either
> > way...
>
> I think it would be cleaner to follow. get_phys_addr_lpae is already a
> bit
Re: [Qemu-devel] [PATCH v2 2/9] ipmi: replace goto by a return statement
On 01/21/2016 11:18 AM, Cédric Le Goater wrote:
Each routine using the IPMI_ADD_RSP_DATA, IPMI_CHECK_CMD_LEN or
IPMI_CHECK_RESERVATION macros needs to define a goto label 'out' to
handle hidden errors. Using directly a return statement as the same
Using a return statement directly has the same
effect and it removes the fact that 'out' needs to be defined.
The code exits in ipmi_sim_handle_command() are a little different
from the rest and a "possible" error in the macro IPMI_ADD_RSP_DATA is
handled before making use of it. This might be a bit excessive as a
minimum response len is currently 300 bytes and the patch checks that
at least 3 are available.
Yeah, it seems a little excessive. The compiler should figure out that
the value is always false and remove the code.
The return in the macro seems to obfuscate the return as much as
the goto, but the code does look a lot neater this way.
Reviewed-by: Corey Minyard
Signed-off-by: Cédric Le Goater
---
hw/ipmi/ipmi_bmc_sim.c | 140 +++--
1 file changed, 41 insertions(+), 99 deletions(-)
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 0a59e539f549..e42c7e86c344 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -258,7 +258,7 @@ struct IPMIBmcSim {
do { \
if (*rsp_len >= max_rsp_len) { \
rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED; \
-goto out; \
+return;\
} \
rsp[(*rsp_len)++] = (b); \
} while (0)
@@ -267,7 +267,7 @@ struct IPMIBmcSim {
#define IPMI_CHECK_CMD_LEN(l) \
if (cmd_len < l) { \
rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID; \
-goto out; \
+return; \
}
/* Check that the reservation in the command is valid. */
@@ -275,7 +275,7 @@ struct IPMIBmcSim {
do { \
if ((cmd[off] | (cmd[off + 1] << 8)) != r) { \
rsp[2] = IPMI_CC_INVALID_RESERVATION; \
-goto out; \
+return;\
} \
} while (0)
@@ -453,14 +453,12 @@ static void gen_event(IPMIBmcSim *ibs, unsigned int sens_num, uint8_t deassert,
}
if (ibs->msg_flags & IPMI_BMC_MSG_FLAG_EVT_BUF_FULL) {
-goto out;
+return;
}
memcpy(ibs->evtbuf, evt, 16);
ibs->msg_flags |= IPMI_BMC_MSG_FLAG_EVT_BUF_FULL;
k->set_atn(s, 1, attn_irq_enabled(ibs));
- out:
-return;
}
static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
@@ -581,6 +579,11 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
/* Set up the response, set the low bit of NETFN. */
/* Note that max_rsp_len must be at least 3 */
+if (max_rsp_len < 3) {
+rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
+goto out;
+}
+
IPMI_ADD_RSP_DATA(cmd[0] | 0x04);
IPMI_ADD_RSP_DATA(cmd[1]);
IPMI_ADD_RSP_DATA(0); /* Assume success */
@@ -698,8 +701,6 @@ static void chassis_capabilities(IPMIBmcSim *ibs,
IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
- out:
-return;
}
static void chassis_status(IPMIBmcSim *ibs,
@@ -711,8 +712,6 @@ static void chassis_status(IPMIBmcSim *ibs,
IPMI_ADD_RSP_DATA(0);
IPMI_ADD_RSP_DATA(0);
IPMI_ADD_RSP_DATA(0);
- out:
-return;
}
static void chassis_control(IPMIBmcSim *ibs,
@@ -746,10 +745,8 @@ static void chassis_control(IPMIBmcSim *ibs,
break;
default:
rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-goto out;
+return;
}
- out:
-return;
}
static void get_device_id(IPMIBmcSim *ibs,
@@ -768,8 +765,6 @@ static void get_device_id(IPMIBmcSim *ibs,
IPMI_ADD_RSP_DATA(ibs->mfg_id[2]);
IPMI_ADD_RSP_DATA(ibs->product_id[0]);
IPMI_ADD_RSP_DATA(ibs->product_id[1]);
- out:
-return;
}
static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
@@ -822,8 +817,6 @@ static void set_bmc_global_enables(IPMIBmcSim *ibs,
{
IPMI_CHECK_CMD_LEN(3);
set_global_enables(ibs, cmd[2]);
- out:
-return;
}
static void get_bmc_global_enables(IPMIBmcSim *ibs,
@@ -832,8 +825,6 @@ static void get_bmc_global_enables(IPMIBmcSim *ibs,
unsigned int max_rsp_len)
{
IPMI_ADD_RSP_DATA(ibs->bmc_global_enables);
- out:
-return;
}
static void clr_msg_flags(IPMIBmcSim *ibs,
@@ -847,8 +838,6 @@ static void clr_msg_flags(IPMI
Re: [Qemu-devel] [PATCH v9 21/37] qapi: Document visitor interfaces, add assertions
Eric Blake writes:
> On 01/21/2016 01:08 PM, Markus Armbruster wrote:
>> All right, this one's a bear. Not because the patch is bad, but because
>> what it tries to do is bloody difficult.
>
> Is there any reasonable split (such as adding some of the assertions in
> earlier patches) that would make it any easier? Or do we just bite the
> bullet and do it?
I think the difficulty is in finding a contract that fits the current
code and makes sense.
Assertions and contract overlap: assertions partly enforce the contract.
Separating the two won't help.
We could try to split along sub-interface boundaries, say scalars,
structs, unions, alternates, lists. Several smaller patches, but to
ensure overall consistency, you have to mentally merge them again.
Doesn't seem helpful.
Let's bite the bullet.
>> Eric Blake writes:
>>
>>> The visitor interface for mapping between QObject/QemuOpts/string
>>> and qapi has formerly been documented only by reading source code,
>>
>> Polite way to say "is scandalously undocumented".
>
> Indeed.
>
>>
>>> making it difficult to propose changes to either scripts/qapi*.py
>>> or to clients without knowing whether those changes would be safe.
>>> This adds documentation, including mentioning when parameters can
>>> be NULL, and where there are still some interface warts that would
>>> be nice to remove. In particular, I have plans to remove
>>> visit_start_union() in a future patch.
>>
>> Suggest
>>
>> The visitor interface for mapping between QObject/QemuOpts/string
>> and QAPI is pretty much undocumented, making changes to visitor
>> core, individual visitors, and users of visitors difficult.
>>
>> Correct this by retrofitting proper contracts. Document some
>> interface warts that would be nice to remove. In particular, I have
>> plans to remove visit_start_union() in a future patch.
>
> Your suggestions here, and elsewhere, are good and will be in my next
> spin. I'll trim to just the places where you have more than just a
> wording suggestion.
>
>
>>> include/qapi/visitor-impl.h | 31 ++-
>>> include/qapi/visitor.h | 196
>>>
>>> qapi/qapi-visit-core.c | 39 -
>>> 3 files changed, 262 insertions(+), 4 deletions(-)
>>>
>>
>> My review probably makes more sense if you skip ahead to visitor.h, then
>> come back here.
>
> If I remember, I'll use -O when generating v10 to force visitor.h first,
> other .h second, and .c last (I don't always remember to do it; maybe I
> should add it into my handy .git alias that I use for firing off long
> series). [I really wish the git people would make it possible to
> automate -O via 'git config', and to make it easier to have a
> per-project preferred order file]
>
>>
>>> +
>>> struct Visitor
>>> {
>>> -/* Must be set */
>>> +/* Must be provided to visit structs (the string visitors do not
>>> + * currently visit structs). */
>>
>> Uh, the string visitors don't decide what gets visited, their users do.
>> The string visitors don't support visiting structs. The restriction
>> needs to be documented, but this isn't the place to do it, is it?
>
> Good point. I think what I will do is split out a separate patch that
> documents, per visitor with limitation, what callers cannot (yet) do
> with that visitor.
Makes sense.
>>> +/* May be NULL; most useful for input visitors. */
>>
>> "Optional" would be a bit terser than "May be NULL".
>>
>> Why is it "most useful for input visitors"? For what it's worth, the
>> dealloc visitor finds it useful, too...
>
> and a later patch adds it to the QemuOpts input visitor (Zoltan's patch
> has been sitting for how many months now?). I'll come up with something.
>
>>
>>> void (*start_implicit_struct)(Visitor *v, void **obj, size_t size,
>>>Error **errp);
>>> /* May be NULL */
>>> void (*end_implicit_struct)(Visitor *v);
>>>
>>> +/* Must be set */
>>> void (*start_list)(Visitor *v, const char *name, Error **errp);
>>> +/* Must be set */
>>> GenericList *(*next_list)(Visitor *v, GenericList **list, Error
>>> **errp);
>>> /* Must be set */
>>> void (*end_list)(Visitor *v);
>>
>> A visitor could omit these two with similar consequences to omitting
>> start_struct() and end_struct(): attempts to visit lists crash then. In
>> fact, the string visitors omitted them until commit 659268f and 69e2556,
>> respectively.
>
> Which is why, as you pointed out, it may be better to document the
> limitations in the string visitor rather than here, and in this file
> maybe just mention at the top something along the lines that "must be
> set" really means that "only needs to be set if your callers are
> expecting a visit to encounter this type; the corresponding crash on
> calling NULL is your hint to write missing functionality in your visitor".
Good idea.
Perhaps say "Must be set for $TYPE visits to work" in
Re: [Qemu-devel] [PATCH v4] qom, qmp, hmp, qapi: create qom-type-prop-list for class properties
On Fri, Jan 22, 2016 at 03:15:55PM +0300, Valentin Rakush wrote: > This patch adds support for qom-type-prop-list command to list object > class properties. A later patch will use this functionality to > implement x86_64-cpu properties. > > Signed-off-by: Valentin Rakush > Cc: Luiz Capitulino > Cc: Eric Blake > Cc: Markus Armbruster > Cc: Andreas Färber > Cc: Daniel P. Berrange > --- > V4: review fixes > - the typename argument in the hmp command changed to be mandatory > V3: commit message fix > - commit message changed to reflect actual command name > V2: Fixes after first review > - changed command name from qom-type-list to qom-type-prop-list > - changed memory allocation from g_malloc0 to g_new0 > - changed parameter name from path to typename > - fixed wordings and comments > - fixed source code formatting > - registered the command in monitor > > hmp-commands.hx | 13 + > hmp.c| 21 + > hmp.h| 1 + > include/qom/object.h | 31 +++ > qapi-schema.json | 19 +++ > qmp-commands.hx | 6 ++ > qmp.c| 32 > qom/object.c | 7 +++ > 8 files changed, 130 insertions(+) Reviewed-by: Daniel P. Berrange Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH v2 8/9] ipmi: add ACPI power and GUID commands
On 01/21/2016 11:18 AM, Cédric Le Goater wrote:
Signed-off-by: Cédric Le Goater
---
Changes since v1:
- added ACPI to command names.
Thanks.
Acked-by: Corey Minyard
hw/ipmi/ipmi_bmc_sim.c | 49 +
1 file changed, 49 insertions(+)
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index e882af3f1b40..53c75cb21c1a 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include "sysemu/sysemu.h"
#include "qemu/timer.h"
#include "hw/ipmi/ipmi.h"
#include "qemu/error-report.h"
@@ -51,6 +52,9 @@
#define IPMI_CMD_GET_DEVICE_ID0x01
#define IPMI_CMD_COLD_RESET 0x02
#define IPMI_CMD_WARM_RESET 0x03
+#define IPMI_CMD_SET_ACPI_POWER_STATE 0x06
+#define IPMI_CMD_GET_ACPI_POWER_STATE 0x07
+#define IPMI_CMD_GET_DEVICE_GUID 0x08
#define IPMI_CMD_RESET_WATCHDOG_TIMER 0x22
#define IPMI_CMD_SET_WATCHDOG_TIMER 0x24
#define IPMI_CMD_GET_WATCHDOG_TIMER 0x25
@@ -200,6 +204,9 @@ struct IPMIBmcSim {
uint8_t restart_cause;
+uint8_t acpi_power_state[2];
+uint8_t uuid[16];
+
IPMISel sel;
IPMISdr sdr;
IPMISensor sensors[MAX_SENSORS];
@@ -828,6 +835,36 @@ static void warm_reset(IPMIBmcSim *ibs,
k->reset(s, false);
}
}
+static void set_acpi_power_state(IPMIBmcSim *ibs,
+ uint8_t *cmd, unsigned int cmd_len,
+ uint8_t *rsp, unsigned int *rsp_len,
+ unsigned int max_rsp_len)
+{
+IPMI_CHECK_CMD_LEN(4);
+ibs->acpi_power_state[0] = cmd[2];
+ibs->acpi_power_state[1] = cmd[3];
+}
+
+static void get_acpi_power_state(IPMIBmcSim *ibs,
+ uint8_t *cmd, unsigned int cmd_len,
+ uint8_t *rsp, unsigned int *rsp_len,
+ unsigned int max_rsp_len)
+{
+IPMI_ADD_RSP_DATA(ibs->acpi_power_state[0]);
+IPMI_ADD_RSP_DATA(ibs->acpi_power_state[1]);
+}
+
+static void get_device_guid(IPMIBmcSim *ibs,
+ uint8_t *cmd, unsigned int cmd_len,
+ uint8_t *rsp, unsigned int *rsp_len,
+ unsigned int max_rsp_len)
+{
+unsigned int i;
+
+for (i = 0; i < 16; i++) {
+IPMI_ADD_RSP_DATA(ibs->uuid[i]);
+}
+}
static void set_bmc_global_enables(IPMIBmcSim *ibs,
uint8_t *cmd, unsigned int cmd_len,
@@ -1609,6 +1646,9 @@ static const IPMICmdHandler app_cmds[] = {
[IPMI_CMD_GET_DEVICE_ID] = get_device_id,
[IPMI_CMD_COLD_RESET] = cold_reset,
[IPMI_CMD_WARM_RESET] = warm_reset,
+[IPMI_CMD_SET_ACPI_POWER_STATE] = set_acpi_power_state,
+[IPMI_CMD_GET_ACPI_POWER_STATE] = get_acpi_power_state,
+[IPMI_CMD_GET_DEVICE_GUID] = get_device_guid,
[IPMI_CMD_SET_BMC_GLOBAL_ENABLES] = set_bmc_global_enables,
[IPMI_CMD_GET_BMC_GLOBAL_ENABLES] = get_bmc_global_enables,
[IPMI_CMD_CLR_MSG_FLAGS] = clr_msg_flags,
@@ -1734,6 +1774,15 @@ static void ipmi_sim_init(Object *obj)
i += len;
}
+ibs->acpi_power_state[0] = 0;
+ibs->acpi_power_state[1] = 0;
+
+if (qemu_uuid_set) {
+memcpy(&ibs->uuid, qemu_uuid, 16);
+} else {
+memset(&ibs->uuid, 0, 16);
+}
+
ipmi_init_sensors_from_sdrs(ibs);
register_cmds(ibs);
