On 9/16/19 11:59 AM, Pawel Wieczorkiewicz wrote:
The payloads' name strings can be of arbitrary size (typically small
with an upper bound of XEN_LIVEPATCH_NAME_SIZE).
Current implementation of the list operation interface allows to copy
names in the XEN_LIVEPATCH_NAME_SIZE chunks regardless of its actual
size and enforces space allocation requirements on userland tools.
To unify and simplify the interface, handle the name strings of
arbitrary size by copying them in adhering chunks to the userland.
In order to let the userland allocate enough space for the incoming
data add an auxiliary interface xc_livepatch_list_get_sizes() that
provides the current number of payload entries and the total size of
all name strings. This is achieved by extending the sysctl list
interface with an extra fields: name_total_size.
The xc_livepatch_list_get_sizes() issues the livepatch sysctl list
operation with the nr field set to 0. In this mode the operation
returns the number of payload entries and calculates the total sizes
for all payloads' names.
When the sysctl operation is issued with a non-zero nr field (for
instance with a value obtained earlier with the prior call to the
xc_livepatch_list_get_sizes()) the new field name_total_size provides
the total size of actually copied data.
Extend the libxc to handle the name back-to-back data transfers.
The xen-livepatch tool is modified to start the list operation with a
call to the xc_livepatch_list_get_sizes() to obtain the actual number
of payloads as well as the necessary space for names.
The tool now always requests the actual number of entries and leaves
the preemption handling to the libxc routine. The libxc still returns
'done' and 'left' parameters with the same semantic allowing the tool
to detect anomalies and react to them. At the moment it is expected
that the tool receives the exact number of entires as requested.
entries
The xen-livepatch tool has been also modified to handle the name
back-to-back transfers correctly.
Signed-off-by: Pawel Wieczorkiewicz <wipa...@amazon.de>
Reviewed-by: Andra-Irina Paraschiv <andra...@amazon.com>
Reviewed-by: Bjoern Doebel <doe...@amazon.de>
Reviewed-by: Martin Pohlack <mpohl...@amazon.de>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
---
Changed since v1:
* added corresponding documentation
docs/misc/livepatch.pandoc | 24 +++++----
tools/libxc/include/xenctrl.h | 49 ++++++++++++------
tools/libxc/xc_misc.c | 100 ++++++++++++++++++++++++++++---------
tools/misc/xen-livepatch.c | 112 ++++++++++++++++++++++--------------------
xen/common/livepatch.c | 31 +++++++++---
xen/include/public/sysctl.h | 15 +++---
6 files changed, 219 insertions(+), 112 deletions(-)
diff --git a/docs/misc/livepatch.pandoc b/docs/misc/livepatch.pandoc
index 406fb79df8..e7bcc70f5a 100644
--- a/docs/misc/livepatch.pandoc
+++ b/docs/misc/livepatch.pandoc
@@ -717,17 +717,20 @@ The caller provides:
* `idx` Index iterator. The index into the hypervisor's payload count. It is
recommended that on first invocation zero be used so that `nr` (which the
hypervisor will update with the remaining payload count) be provided.
- Also the hypervisor will provide `version` with the most current value.
+ Also the hypervisor will provide `version` with the most current value and
+ calculated total size for all payloads' names.
* `nr` The max number of entries to populate. Can be zero which will result
in the hypercall being a probing one and return the number of payloads
(and update the `version`).
* `pad` - *MUST* be zero.
* `status` Virtual address of where to write `struct xen_livepatch_status`
structures. Caller *MUST* allocate up to `nr` of them.
- * `name` - Virtual address of where to write the unique name of the payload.
- Caller *MUST* allocate up to `nr` of them. Each *MUST* be of
- **XEN_LIVEPATCH_NAME_SIZE** size. Note that **XEN_LIVEPATCH_NAME_SIZE**
includes
- the NUL terminator.
+ * `name` - Virtual address of where to write the unique name of the payloads.
+ Caller *MUST* allocate enough space to be able to store all received data
+ (i.e. total allocated space *MUST* match the `name_total_size` value
+ provided by the hypervisor). Individual payload name cannot be longer than
+ **XEN_LIVEPATCH_NAME_SIZE** bytes. Note that **XEN_LIVEPATCH_NAME_SIZE**
+ includes the NUL terminator.
* `len` - Virtual address of where to write the length of each unique name
of the payload. Caller *MUST* allocate up to `nr` of them. Each *MUST* be
of sizeof(uint32_t) (4 bytes).
@@ -736,7 +739,8 @@ If the hypercall returns an positive number, it is the
number (upto `nr`
provided to the hypercall) of the payloads returned, along with `nr` updated
with the number of remaining payloads, `version` updated (it may be the same
across hypercalls - if it varies the data is stale and further calls could
-fail). The `status`, `name`, and `len` are updated at their designed index
+fail) and the `name_total_size` containing total size of transfered data for
transferred
+the array. The `status`, `name`, and `len` are updated at their designed index
value (`idx`) with the returned value of data.
If the hypercall returns -XEN_E2BIG the `nr` is too big and should be
@@ -775,11 +779,13 @@ The structure is as follow:
amount of payloads and
version.
OUT: How many payloads
left. */
uint32_t pad; /* IN: Must be zero. */
+ uint64_t name_total_size; /* OUT: Total size of all
transfer names */
XEN_GUEST_HANDLE_64(xen_livepatch_status_t) status; /* OUT. Must
have enough
space allocate for nr of
them. */
- XEN_GUEST_HANDLE_64(char) id; /* OUT: Array of names. Each
member
- MUST
XEN_LIVEPATCH_NAME_SIZE in size.
- Must have nr of them. */
+ XEN_GUEST_HANDLE_64(char) name; /* OUT: Array of names. Each
member
+ may have an arbitrary
length up to
+ XEN_LIVEPATCH_NAME_SIZE
bytes. Must have
+ nr of them. */
XEN_GUEST_HANDLE_64(uint32) len; /* OUT: Array of lengths of
name's.
Must have nr of them. */
};
diff --git a/tools/misc/xen-livepatch.c b/tools/misc/xen-livepatch.c
index a37b2457ff..8ac3d567fc 100644
--- a/tools/misc/xen-livepatch.c
+++ b/tools/misc/xen-livepatch.c
@@ -64,14 +64,14 @@ static const char *state2str(unsigned int state)
return names[state];
}
-/* This value was choosen adhoc. It could be 42 too. */
-#define MAX_LEN 11
static int list_func(int argc, char *argv[])
{
- unsigned int idx, done, left, i;
+ unsigned int nr, done, left, i;
xen_livepatch_status_t *info = NULL;
char *name = NULL;
uint32_t *len = NULL;
+ uint64_t name_total_size;
+ off_t name_off;
If name_total_size becomes 32-bit, then I think you can replace the few
usages of off_t with just a uint32_t (it doesn't need to be signed).
Otherwise looks good to me.
Thanks,
--
Ross Lagerwall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
