Re: [PATCH v2] tools: acrntrace: Add ring buffer mode
toggle quoted message
Show quoted text
There is another paragraph later in the README.rst files that says:
512MB storage space will be reserved by default. This ensures that acrntrace
will exit automatically when the free storage space on the disk is less than
reserved space. Reserved space on the disk is configurable through '-r'.
This should be updated too, shouldn't it?
Thanks,
Geoffroy
-----Original Message-----Suggestion: s/when full/when reaching the end of the buffer
From: acrn-dev@... [mailto:acrn-dev@...]
On Behalf Of Zhipeng Gong
Sent: Monday, August 13, 2018 3:31 PM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH v2] tools: acrntrace: Add ring buffer mode
When running longevity test and capturing acrntrace, generated acrntrace
files sizes are too big.
Sometimes we don't care very old trace. This patch adds ring buffer mode,
fixes acrntrace file size and overwrites the oldest trace with new trace when
the buffer is full.
v2:
- update README.rst
Signed-off-by: Zhipeng Gong <zhipeng.gong@...>
---
tools/acrntrace/README.rst | 4 ++--
tools/acrntrace/acrntrace.c | 44
+++++++++++++++++++++++++++++++++++++++++---
tools/acrntrace/acrntrace.h | 1 +
3 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/tools/acrntrace/README.rst b/tools/acrntrace/README.rst index
433f161..f25c5b0 100644
--- a/tools/acrntrace/README.rst
+++ b/tools/acrntrace/README.rst
@@ -21,8 +21,8 @@ Options:
-i period specify polling interval in milliseconds [1-999]
-t max_time max time to capture trace data (in second)
-c clear the buffered old data
--r free_space amount of free space (in MB) remaining on the disk
- before acrntrace stops
+-r ring_buffer_size ring buffer size (in MB), which enables ring buffer
+ mode and overwrites old trace with new trace
+when full
There is another paragraph later in the README.rst files that says:
512MB storage space will be reserved by default. This ensures that acrntrace
will exit automatically when the free storage space on the disk is less than
reserved space. Reserved space on the disk is configurable through '-r'.
This should be updated too, shouldn't it?
Thanks,
Geoffroy
The ``acrntrace_format.py`` is a offline tool for parsing trace data (as output
by acrntrace) to human-readable formats based on given format.
diff --git a/tools/acrntrace/acrntrace.c b/tools/acrntrace/acrntrace.c index
39fda35..e4b3899 100644
--- a/tools/acrntrace/acrntrace.c
+++ b/tools/acrntrace/acrntrace.c
@@ -37,16 +37,19 @@ static char trace_file_dir[TRACE_FILE_DIR_LEN];
static reader_struct *reader;
static int pcpu_num = 0;
+static int ring_buffer_size = 0;
static void display_usage(void)
{
printf("acrntrace - tool to collect ACRN trace data\n"
- "[Usage] acrntrace [-i] [period in msec] [-ch]\n\n"
+ "[Usage] acrntrace [-i] [period in msec] [-r] [ring buffer size] [-
ch]\n\n"
"[Options]\n"
"\t-h: print this message\n"
"\t-i: period_in_ms: specify polling interval [1-999]\n"
"\t-t: max time to capture trace data (in second)\n"
- "\t-c: clear the buffered old data\n");
+ "\t-c: clear the buffered old data\n"
+ "\t-r: ring_buffer_size (in MB), which enables ring buffer mode"
+ " and overwrites old trace with new trace when full\n");
}
static void timer_handler(union sigval sv) @@ -113,6 +116,15 @@ static int
parse_opt(int argc, char *argv[])
timeout = ret;
pr_dbg("Capture trace data for at most %ds\n", ret);
break;
+ case 'r':
+ ret = atoi(optarg);
+ if (ret <= 0) {
+ pr_err("'-r' require integer greater than 0\n");
+ return -EINVAL;
+ }
+ ring_buffer_size = ret * 1024 * 1024;
+ pr_dbg("Ring buffer size is %dM\n", ret);
+ break;
case 'c':
flags |= FLAG_CLEAR_BUF;
break;
@@ -206,6 +218,15 @@ static void reader_fn(param_t * param)
trace_ev_t e;
struct statvfs stat;
uint64_t freespace;
+ int pos = 0;
+
+ if (ring_buffer_size != 0) {
+ param->buffer = malloc(ring_buffer_size);
+ if (!param->buffer) {
+ perror("Failed to allocate ring buffer\n");
+ return;
+ }
+ }
pr_dbg("reader thread[%lu] created for FILE*[0x%p]\n",
pthread_self(), fp);
@@ -219,7 +240,13 @@ static void reader_fn(param_t * param)
while (1) {
do {
- ret = sbuf_write(fd, sbuf);
+ if (ring_buffer_size != 0) {
+ ret = sbuf_get(sbuf, param->buffer + pos);
+ pos += sbuf->ele_size;
+ if (pos + sbuf->ele_size > ring_buffer_size)
+ pos = 0;
+ } else
+ ret = sbuf_write(fd, sbuf);
} while (ret > 0);
usleep(period);
@@ -284,6 +311,17 @@ static void destory_reader(reader_struct * reader)
reader->thrd = 0;
}
+ if (ring_buffer_size != 0 && reader->param.buffer) {
+ int ret;
+
+ ret = write(reader->param.trace_fd, reader->param.buffer,
+ ring_buffer_size);
+ if (ret != ring_buffer_size) {
+ perror("Failed to write ring buffer\n");
+ }
+ free(reader->param.buffer);
+ }
+
if (reader->param.sbuf) {
munmap(reader->param.sbuf, MMAP_SIZE);
reader->param.sbuf = NULL;
diff --git a/tools/acrntrace/acrntrace.h b/tools/acrntrace/acrntrace.h index
ea54f4d..6615157 100644
--- a/tools/acrntrace/acrntrace.h
+++ b/tools/acrntrace/acrntrace.h
@@ -76,6 +76,7 @@ typedef struct {
int trace_fd;
shared_buf_t *sbuf;
pthread_mutex_t *sbuf_lock;
+ uint8_t *buffer;
} param_t;
typedef struct {
--
2.7.4
Re: [PATCH 4/6] DM USB: xHCI: refine port assignment logic
Yu Wang
On 18-08-13 16:48:33, Wu, Xiaoguang wrote:
The variable native_assign_ports in struct pci_xhci_vdev is usedFor "= 0" and "< 0", please define some macros...
to record wether certain root hub port in SOS is assgned to UOS.
The logic is zero reserve and nonzero assignement.
In this patch, the meaning of native_assign_ports is expand to the
following logic:
native_assign_ports[x][y] = 0: native port x-y is not assigned.
native_assign_ports[x][y] < 0: native port x-y is assigned but
currently no virtual device attached.
native_assign_ports[x][y] > 0: native paort x-y is assigned and
virtual device attached to virtual port whose number
is stored in native_assign_ports[x][y].
Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index f12126b..2df7ce3 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -373,7 +373,16 @@ struct pci_xhci_vdev {
int (*excap_write)(struct pci_xhci_vdev *, uint64_t, uint64_t);
int usb2_port_start;
int usb3_port_start;
- uint8_t *native_assign_ports[USB_NATIVE_NUM_BUS];
+
+ /* native_assign_ports is used to record the assigment information:
+ * native_assign_ports[x][y] = 0: native port x-y is not assigned.
+ * native_assign_ports[x][y] < 0: native port x-y is assigned but
+ * currently no virtual device attached.
+ * native_assign_ports[x][y] > 0: native paort x-y is assigned and
+ * virtual device attached to virtual port whose number
+ * is stored in native_assign_ports[x][y].
+ */
+ int8_t *native_assign_ports[USB_NATIVE_NUM_BUS];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};
@@ -3365,14 +3374,14 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
if (!xdev->native_assign_ports[bus]) {
xdev->native_assign_ports[bus] = calloc(USB_NATIVE_NUM_PORT,
- sizeof(uint8_t));
+ sizeof(int8_t));
if (!xdev->native_assign_ports[bus]) {
rc = -3;
goto errout;
}
}
+ xdev->native_assign_ports[bus][port] = -1;
- xdev->native_assign_ports[bus][port] = 1;
errout:
if (rc)
UPRINTF(LWRN, "%s fails, rc=%d\r\n", __func__, rc);
--
2.7.4
Re: [PATCH 3/6] DM USB: introduce struct usb_native_devinfo
Yu Wang
On 18-08-13 16:48:32, Wu, Xiaoguang wrote:
struct usb_dev which included native usb device information. Why define
a new one? Should we refine the struct usb_dev to replace these native
description members?
Current design cannot get physical USB device information withoutdificulties?
the creation of pci_xhci_dev_emu. This brings some dificulties in
certain situations, hence struct usb_native_devinfo is introducedThen usb_dev_info is not useful now? Should we clear it?
to describe neccessary information to solve this trouble.
Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 31 ++++++++++++-------------------
devicemodel/hw/platform/usb_pmapper.c | 14 +++++++++++++-
devicemodel/include/usb_core.h | 10 ++++++++++
3 files changed, 35 insertions(+), 20 deletions(-)
diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index 490219a..f12126b 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -469,23 +469,22 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
struct pci_xhci_dev_emu *de;
struct pci_xhci_vdev *xdev;
struct usb_devemu *ue;
+ struct usb_native_devinfo *di;
int port_start, port_end;
int slot_start, slot_end;
int port, slot;
void *ud;
- uint8_t native_bus, native_pid, native_port;
- uint16_t native_vid;
- int native_speed;
int intr = 1;
xdev = hci_data;
+ di = dev_data;
assert(xdev);
assert(dev_data);
assert(xdev->devices);
assert(xdev->slots);
- de = pci_xhci_dev_create(xdev, dev_data);
+ de = pci_xhci_dev_create(xdev, di->priv_data);
if (!de) {
UPRINTF(LFTL, "fail to create device\r\n");
return -1;
@@ -499,26 +498,20 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
assert(ue);
/* print physical information about new device */
- ue->ue_info(ud, USB_INFO_BUS, &native_bus, sizeof(native_bus));
- ue->ue_info(ud, USB_INFO_PORT, &native_port, sizeof(native_port));
- ue->ue_info(ud, USB_INFO_VID, &native_vid, sizeof(native_vid));
- ue->ue_info(ud, USB_INFO_PID, &native_pid, sizeof(native_pid));
- ue->ue_info(ud, USB_INFO_SPEED, &native_speed, sizeof(native_speed));
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",This new structure is confused to me. There has already one structure
- native_vid, native_pid, native_bus, native_port);
+ di->vid, di->pid, di->bus, di->port);
- if (!xdev->native_assign_ports[native_bus] ||
- !xdev->native_assign_ports[native_bus][native_port]) {
+ if (!xdev->native_assign_ports[di->bus] ||
+ !xdev->native_assign_ports[di->bus][di->port]) {
UPRINTF(LDBG, "%04x:%04x %d-%d doesn't belong to this vm, bye."
- "\r\n", native_vid, native_pid, native_bus,
- native_port);
+ "\r\n", di->vid, di->pid, di->bus, di->port);
goto errout;
}
- UPRINTF(LDBG, "%04x:%04x %d-%d belong to this vm.\r\n", native_vid,
- native_pid, native_bus, native_port);
+ UPRINTF(LDBG, "%04x:%04x %d-%d belong to this vm.\r\n", di->vid,
+ di->pid, di->bus, di->port);
- if (ue->ue_usbver == 2)
+ if (di->bcd < 0x300)
port_start = xdev->usb2_port_start;
else
port_start = xdev->usb3_port_start;
@@ -550,11 +543,11 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
pci_xhci_reset_slot(xdev, slot);
UPRINTF(LDBG, "%X:%X %d-%d locates in slot %d port %d.\r\n",
- native_vid, native_pid, native_bus, native_port,
+ di->vid, di->pid, di->bus, di->port,
slot, port);
/* Trigger port change event for the arriving device */
- if (pci_xhci_port_connect(xdev, port, native_speed, intr))
+ if (pci_xhci_port_connect(xdev, port, di->speed, intr))
UPRINTF(LFTL, "fail to report port event\n");
return 0;
diff --git a/devicemodel/hw/platform/usb_pmapper.c b/devicemodel/hw/platform/usb_pmapper.c
index 9f57f1c..a9c8608 100644
--- a/devicemodel/hw/platform/usb_pmapper.c
+++ b/devicemodel/hw/platform/usb_pmapper.c
@@ -1020,6 +1020,9 @@ static int
usb_dev_native_sys_conn_cb(struct libusb_context *ctx, struct libusb_device
*ldev, libusb_hotplug_event event, void *pdata)
{
+ struct libusb_device_descriptor d;
+ struct usb_native_devinfo di;
+
UPRINTF(LDBG, "connect event\r\n");
if (!ctx || !ldev) {
@@ -1027,8 +1030,17 @@ usb_dev_native_sys_conn_cb(struct libusb_context *ctx, struct libusb_device
return -1;
}
+ libusb_get_device_descriptor(ldev, &d);
+ di.bus = libusb_get_bus_number(ldev);
+ di.speed = libusb_get_device_speed(ldev);
+ di.port = libusb_get_port_number(ldev);
+ di.pid = d.idProduct;
+ di.vid = d.idVendor;
+ di.bcd = d.bcdUSB;
+ di.priv_data = ldev;
+
if (g_ctx.conn_cb)
- g_ctx.conn_cb(g_ctx.hci_data, ldev);
+ g_ctx.conn_cb(g_ctx.hci_data, &di);
return 0;
}
diff --git a/devicemodel/include/usb_core.h b/devicemodel/include/usb_core.h
index 3a9648a..8727b8b 100644
--- a/devicemodel/include/usb_core.h
+++ b/devicemodel/include/usb_core.h
@@ -163,6 +163,16 @@ struct usb_data_xfer {
pthread_mutex_t mtx;
};
+struct usb_native_devinfo {
+ int speed;
+ uint8_t bus;
+ uint8_t port;
+ uint16_t bcd;
+ uint16_t pid;
+ uint16_t vid;
+ void *priv_data;
+};
struct usb_dev which included native usb device information. Why define
a new one? Should we refine the struct usb_dev to replace these native
description members?
+
enum USB_ERRCODE {
USB_ACK,
USB_NAK,
--
2.7.4
[PATCH v2] tools: acrntrace: Add ring buffer mode
Zhipeng Gong <zhipeng.gong@...>
When running longevity test and capturing acrntrace, generated acrntrace
files sizes are too big.
Sometimes we don't care very old trace. This patch adds ring buffer
mode, fixes acrntrace file size and overwrites the oldest trace with new
trace when the buffer is full.
v2:
- update README.rst
Signed-off-by: Zhipeng Gong <zhipeng.gong@...>
---
tools/acrntrace/README.rst | 4 ++--
tools/acrntrace/acrntrace.c | 44 +++++++++++++++++++++++++++++++++++++++++---
tools/acrntrace/acrntrace.h | 1 +
3 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/tools/acrntrace/README.rst b/tools/acrntrace/README.rst
index 433f161..f25c5b0 100644
--- a/tools/acrntrace/README.rst
+++ b/tools/acrntrace/README.rst
@@ -21,8 +21,8 @@ Options:
-i period specify polling interval in milliseconds [1-999]
-t max_time max time to capture trace data (in second)
-c clear the buffered old data
--r free_space amount of free space (in MB) remaining on the disk
- before acrntrace stops
+-r ring_buffer_size ring buffer size (in MB), which enables ring buffer
+ mode and overwrites old trace with new trace when full
The ``acrntrace_format.py`` is a offline tool for parsing trace data (as output
by acrntrace) to human-readable formats based on given format.
diff --git a/tools/acrntrace/acrntrace.c b/tools/acrntrace/acrntrace.c
index 39fda35..e4b3899 100644
--- a/tools/acrntrace/acrntrace.c
+++ b/tools/acrntrace/acrntrace.c
@@ -37,16 +37,19 @@ static char trace_file_dir[TRACE_FILE_DIR_LEN];
static reader_struct *reader;
static int pcpu_num = 0;
+static int ring_buffer_size = 0;
static void display_usage(void)
{
printf("acrntrace - tool to collect ACRN trace data\n"
- "[Usage] acrntrace [-i] [period in msec] [-ch]\n\n"
+ "[Usage] acrntrace [-i] [period in msec] [-r] [ring buffer size] [-ch]\n\n"
"[Options]\n"
"\t-h: print this message\n"
"\t-i: period_in_ms: specify polling interval [1-999]\n"
"\t-t: max time to capture trace data (in second)\n"
- "\t-c: clear the buffered old data\n");
+ "\t-c: clear the buffered old data\n"
+ "\t-r: ring_buffer_size (in MB), which enables ring buffer mode"
+ " and overwrites old trace with new trace when full\n");
}
static void timer_handler(union sigval sv)
@@ -113,6 +116,15 @@ static int parse_opt(int argc, char *argv[])
timeout = ret;
pr_dbg("Capture trace data for at most %ds\n", ret);
break;
+ case 'r':
+ ret = atoi(optarg);
+ if (ret <= 0) {
+ pr_err("'-r' require integer greater than 0\n");
+ return -EINVAL;
+ }
+ ring_buffer_size = ret * 1024 * 1024;
+ pr_dbg("Ring buffer size is %dM\n", ret);
+ break;
case 'c':
flags |= FLAG_CLEAR_BUF;
break;
@@ -206,6 +218,15 @@ static void reader_fn(param_t * param)
trace_ev_t e;
struct statvfs stat;
uint64_t freespace;
+ int pos = 0;
+
+ if (ring_buffer_size != 0) {
+ param->buffer = malloc(ring_buffer_size);
+ if (!param->buffer) {
+ perror("Failed to allocate ring buffer\n");
+ return;
+ }
+ }
pr_dbg("reader thread[%lu] created for FILE*[0x%p]\n",
pthread_self(), fp);
@@ -219,7 +240,13 @@ static void reader_fn(param_t * param)
while (1) {
do {
- ret = sbuf_write(fd, sbuf);
+ if (ring_buffer_size != 0) {
+ ret = sbuf_get(sbuf, param->buffer + pos);
+ pos += sbuf->ele_size;
+ if (pos + sbuf->ele_size > ring_buffer_size)
+ pos = 0;
+ } else
+ ret = sbuf_write(fd, sbuf);
} while (ret > 0);
usleep(period);
@@ -284,6 +311,17 @@ static void destory_reader(reader_struct * reader)
reader->thrd = 0;
}
+ if (ring_buffer_size != 0 && reader->param.buffer) {
+ int ret;
+
+ ret = write(reader->param.trace_fd, reader->param.buffer,
+ ring_buffer_size);
+ if (ret != ring_buffer_size) {
+ perror("Failed to write ring buffer\n");
+ }
+ free(reader->param.buffer);
+ }
+
if (reader->param.sbuf) {
munmap(reader->param.sbuf, MMAP_SIZE);
reader->param.sbuf = NULL;
diff --git a/tools/acrntrace/acrntrace.h b/tools/acrntrace/acrntrace.h
index ea54f4d..6615157 100644
--- a/tools/acrntrace/acrntrace.h
+++ b/tools/acrntrace/acrntrace.h
@@ -76,6 +76,7 @@ typedef struct {
int trace_fd;
shared_buf_t *sbuf;
pthread_mutex_t *sbuf_lock;
+ uint8_t *buffer;
} param_t;
typedef struct {
--
2.7.4
files sizes are too big.
Sometimes we don't care very old trace. This patch adds ring buffer
mode, fixes acrntrace file size and overwrites the oldest trace with new
trace when the buffer is full.
v2:
- update README.rst
Signed-off-by: Zhipeng Gong <zhipeng.gong@...>
---
tools/acrntrace/README.rst | 4 ++--
tools/acrntrace/acrntrace.c | 44 +++++++++++++++++++++++++++++++++++++++++---
tools/acrntrace/acrntrace.h | 1 +
3 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/tools/acrntrace/README.rst b/tools/acrntrace/README.rst
index 433f161..f25c5b0 100644
--- a/tools/acrntrace/README.rst
+++ b/tools/acrntrace/README.rst
@@ -21,8 +21,8 @@ Options:
-i period specify polling interval in milliseconds [1-999]
-t max_time max time to capture trace data (in second)
-c clear the buffered old data
--r free_space amount of free space (in MB) remaining on the disk
- before acrntrace stops
+-r ring_buffer_size ring buffer size (in MB), which enables ring buffer
+ mode and overwrites old trace with new trace when full
The ``acrntrace_format.py`` is a offline tool for parsing trace data (as output
by acrntrace) to human-readable formats based on given format.
diff --git a/tools/acrntrace/acrntrace.c b/tools/acrntrace/acrntrace.c
index 39fda35..e4b3899 100644
--- a/tools/acrntrace/acrntrace.c
+++ b/tools/acrntrace/acrntrace.c
@@ -37,16 +37,19 @@ static char trace_file_dir[TRACE_FILE_DIR_LEN];
static reader_struct *reader;
static int pcpu_num = 0;
+static int ring_buffer_size = 0;
static void display_usage(void)
{
printf("acrntrace - tool to collect ACRN trace data\n"
- "[Usage] acrntrace [-i] [period in msec] [-ch]\n\n"
+ "[Usage] acrntrace [-i] [period in msec] [-r] [ring buffer size] [-ch]\n\n"
"[Options]\n"
"\t-h: print this message\n"
"\t-i: period_in_ms: specify polling interval [1-999]\n"
"\t-t: max time to capture trace data (in second)\n"
- "\t-c: clear the buffered old data\n");
+ "\t-c: clear the buffered old data\n"
+ "\t-r: ring_buffer_size (in MB), which enables ring buffer mode"
+ " and overwrites old trace with new trace when full\n");
}
static void timer_handler(union sigval sv)
@@ -113,6 +116,15 @@ static int parse_opt(int argc, char *argv[])
timeout = ret;
pr_dbg("Capture trace data for at most %ds\n", ret);
break;
+ case 'r':
+ ret = atoi(optarg);
+ if (ret <= 0) {
+ pr_err("'-r' require integer greater than 0\n");
+ return -EINVAL;
+ }
+ ring_buffer_size = ret * 1024 * 1024;
+ pr_dbg("Ring buffer size is %dM\n", ret);
+ break;
case 'c':
flags |= FLAG_CLEAR_BUF;
break;
@@ -206,6 +218,15 @@ static void reader_fn(param_t * param)
trace_ev_t e;
struct statvfs stat;
uint64_t freespace;
+ int pos = 0;
+
+ if (ring_buffer_size != 0) {
+ param->buffer = malloc(ring_buffer_size);
+ if (!param->buffer) {
+ perror("Failed to allocate ring buffer\n");
+ return;
+ }
+ }
pr_dbg("reader thread[%lu] created for FILE*[0x%p]\n",
pthread_self(), fp);
@@ -219,7 +240,13 @@ static void reader_fn(param_t * param)
while (1) {
do {
- ret = sbuf_write(fd, sbuf);
+ if (ring_buffer_size != 0) {
+ ret = sbuf_get(sbuf, param->buffer + pos);
+ pos += sbuf->ele_size;
+ if (pos + sbuf->ele_size > ring_buffer_size)
+ pos = 0;
+ } else
+ ret = sbuf_write(fd, sbuf);
} while (ret > 0);
usleep(period);
@@ -284,6 +311,17 @@ static void destory_reader(reader_struct * reader)
reader->thrd = 0;
}
+ if (ring_buffer_size != 0 && reader->param.buffer) {
+ int ret;
+
+ ret = write(reader->param.trace_fd, reader->param.buffer,
+ ring_buffer_size);
+ if (ret != ring_buffer_size) {
+ perror("Failed to write ring buffer\n");
+ }
+ free(reader->param.buffer);
+ }
+
if (reader->param.sbuf) {
munmap(reader->param.sbuf, MMAP_SIZE);
reader->param.sbuf = NULL;
diff --git a/tools/acrntrace/acrntrace.h b/tools/acrntrace/acrntrace.h
index ea54f4d..6615157 100644
--- a/tools/acrntrace/acrntrace.h
+++ b/tools/acrntrace/acrntrace.h
@@ -76,6 +76,7 @@ typedef struct {
int trace_fd;
shared_buf_t *sbuf;
pthread_mutex_t *sbuf_lock;
+ uint8_t *buffer;
} param_t;
typedef struct {
--
2.7.4
Re: [PATCH 2/6] DM USB: xHCI: refine xHCI PORTSC Register related functions
Yu Wang
On 18-08-13 16:48:31, Wu, Xiaoguang wrote:
to pci_xhci_port_disconnect directly..
PORTSC (Port Status and Control Register) register play a veryCan we remove intr variable? Is hasn't changed in this function and pass
important role in USB sub-system. This patch is used to refine
related manipulation functions.
Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 76 ++++++++++++++++++++++-------------------------
1 file changed, 35 insertions(+), 41 deletions(-)
diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index 792d930..490219a 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -439,11 +439,14 @@ static void pci_xhci_update_ep_ring(struct pci_xhci_vdev *xdev,
struct xhci_endp_ctx *ep_ctx,
uint32_t streamid, uint64_t ringaddr,
int ccs);
-static void pci_xhci_init_port(struct pci_xhci_vdev *xdev, int portn);
+static void pci_xhci_port_init(struct pci_xhci_vdev *xdev, int portn);
+static int pci_xhci_port_connect(struct pci_xhci_vdev *xdev, int port,
+ int usb_speed, int intr);
+static int pci_xhci_port_disconnect(struct pci_xhci_vdev *xdev, int port,
+ int intr);
static struct pci_xhci_dev_emu *pci_xhci_dev_create(struct pci_xhci_vdev *
xdev, void *dev_data);
static void pci_xhci_dev_destroy(struct pci_xhci_dev_emu *de);
-static int pci_xhci_port_chg(struct pci_xhci_vdev *xdev, int port, int conn);
static void pci_xhci_set_evtrb(struct xhci_trb *evtrb, uint64_t port,
uint32_t errcode, uint32_t evtype);
static int pci_xhci_xfer_complete(struct pci_xhci_vdev *xdev,
@@ -472,6 +475,8 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
void *ud;
uint8_t native_bus, native_pid, native_port;
uint16_t native_vid;
+ int native_speed;
+ int intr = 1;
to pci_xhci_port_disconnect directly..
Ditto.
xdev = hci_data;
@@ -498,6 +503,7 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
ue->ue_info(ud, USB_INFO_PORT, &native_port, sizeof(native_port));
ue->ue_info(ud, USB_INFO_VID, &native_vid, sizeof(native_vid));
ue->ue_info(ud, USB_INFO_PID, &native_pid, sizeof(native_pid));
+ ue->ue_info(ud, USB_INFO_SPEED, &native_speed, sizeof(native_speed));
UPRINTF(LDBG, "%04x:%04x %d-%d connecting.\r\n",
native_vid, native_pid, native_bus, native_port);
@@ -543,14 +549,12 @@ pci_xhci_native_usb_dev_conn_cb(void *hci_data, void *dev_data)
xdev->ndevices++;
pci_xhci_reset_slot(xdev, slot);
- pci_xhci_init_port(xdev, port);
-
UPRINTF(LDBG, "%X:%X %d-%d locates in slot %d port %d.\r\n",
native_vid, native_pid, native_bus, native_port,
slot, port);
/* Trigger port change event for the arriving device */
- if (pci_xhci_port_chg(xdev, port, 1))
+ if (pci_xhci_port_connect(xdev, port, native_speed, intr))
UPRINTF(LFTL, "fail to report port event\n");
return 0;
@@ -566,6 +570,7 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
struct pci_xhci_dev_emu *edev;
struct usb_dev *udev;
uint8_t port, native_port;
+ int intr = 1;
What is mean of intr? From this patch, it almost useless..
assert(hci_data);
assert(dev_data);
@@ -595,7 +600,7 @@ pci_xhci_native_usb_dev_disconn_cb(void *hci_data, void *dev_data)
}
UPRINTF(LDBG, "report virtual port %d status\r\n", port);
- if (pci_xhci_port_chg(xdev, port, 0)) {
+ if (pci_xhci_port_disconnect(xdev, port, intr)) {
UPRINTF(LFTL, "fail to report event\r\n");
return -1;
}
@@ -784,32 +789,29 @@ pci_xhci_convert_speed(int lspeed)
}
static int
-pci_xhci_port_chg(struct pci_xhci_vdev *xdev, int port, int conn)
+pci_xhci_port_change(struct pci_xhci_vdev *xdev, int port, int usb_speed,
+ int conn, int intr)
{
int speed, error;
struct xhci_trb evtrb;
struct pci_xhci_portregs *reg;
- struct pci_xhci_dev_emu *dev;
assert(xdev != NULL);
reg = XHCI_PORTREG_PTR(xdev, port);
- dev = XHCI_DEVINST_PTR(xdev, port);
- if (!dev || !dev->dev_ue || !reg) {
- UPRINTF(LWRN, "find nullptr with port %d\r\n", port);
- return -1;
- }
-
if (conn == 0) {
reg->portsc &= ~XHCI_PS_CCS;
reg->portsc |= (XHCI_PS_CSC |
XHCI_PS_PLS_SET(UPS_PORT_LS_RX_DET));
} else {
- speed = pci_xhci_convert_speed(dev->dev_ue->ue_usbspeed);
+ speed = pci_xhci_convert_speed(usb_speed);
reg->portsc = XHCI_PS_CCS | XHCI_PS_PP | XHCI_PS_CSC;
reg->portsc |= XHCI_PS_SPEED_SET(speed);
}
+ if (!intr)
+ return 0;
+Let's keep pci_xhci_init_port..
/* make an event for the guest OS */
pci_xhci_set_evtrb(&evtrb,
port,
@@ -825,6 +827,20 @@ pci_xhci_port_chg(struct pci_xhci_vdev *xdev, int port, int conn)
return (error == XHCI_TRB_ERROR_SUCCESS) ? 0 : -1;
}
+static int
+pci_xhci_port_connect(struct pci_xhci_vdev *xdev, int port, int usb_speed,
+ int intr)
+{
+ return pci_xhci_port_change(xdev, port, usb_speed, 1, intr);
+}
+
+static int
+pci_xhci_port_disconnect(struct pci_xhci_vdev *xdev, int port, int intr)
+{
+ /* for disconnect, the speed is useless */
+ return pci_xhci_port_change(xdev, port, 0, 0, intr);
+}
+
static void
pci_xhci_set_evtrb(struct xhci_trb *evtrb, uint64_t port, uint32_t errcode,
uint32_t evtype)
@@ -3206,32 +3222,10 @@ pci_xhci_reset_port(struct pci_xhci_vdev *xdev, int portn, int warm)
}
static void
-pci_xhci_init_port(struct pci_xhci_vdev *xdev, int portn)
+pci_xhci_port_init(struct pci_xhci_vdev *xdev, int portn)
{
- struct pci_xhci_portregs *port;
- struct pci_xhci_dev_emu *dev;
-
- port = XHCI_PORTREG_PTR(xdev, portn);
- dev = XHCI_DEVINST_PTR(xdev, portn);
- if (dev) {
- port->portsc = XHCI_PS_CCS | /* connected */
- XHCI_PS_PP; /* port power */
-
- if (dev->dev_ue->ue_usbver == 2) {
- port->portsc |= XHCI_PS_PLS_SET(UPS_PORT_LS_POLL) |
- XHCI_PS_SPEED_SET(dev->dev_ue->ue_usbspeed);
- } else {
- port->portsc |= XHCI_PS_PLS_SET(UPS_PORT_LS_U0) |
- XHCI_PS_PED | /* enabled */
- XHCI_PS_SPEED_SET(dev->dev_ue->ue_usbspeed);
- }
-
- UPRINTF(LDBG, "Init port %d 0x%x\n", portn, port->portsc);
- } else {
- port->portsc = XHCI_PS_PLS_SET(UPS_PORT_LS_RX_DET) | XHCI_PS_PP;
- UPRINTF(LDBG, "Init empty port %d 0x%x\n",
- portn, port->portsc);
- }
+ XHCI_PORTREG_PTR(xdev, portn)->portsc =
+ XHCI_PS_PLS_SET(UPS_PORT_LS_RX_DET) | XHCI_PS_PP;
}
static int
@@ -3567,7 +3561,7 @@ pci_xhci_parse_opts(struct pci_xhci_vdev *xdev, char *opts)
/* do not use the zero index element */
for (i = 1; i <= XHCI_MAX_DEVS; i++)
- pci_xhci_init_port(xdev, i);
+ pci_xhci_port_init(xdev, i);
errout:
if (rc) {
--
2.7.4
Re: [PATCH v2] tools:acrn-crashlog: Document of configuration file
Thanks!
@David, can you take a look at this when you get a chance?
Geoffroy
@David, can you take a look at this when you get a chance?
Patch v3 have been confirmed by local doc building system. I put theThanks,
"images/dot files" into conf.rst, since the doc building system collects only rst
files to doc directory.
Geoffroy
-----Original Message-----
From: acrn-dev@... [mailto:acrn-dev@...]
On Behalf Of Liu xinwu
Sent: Monday, August 13, 2018 4:38 AM
To: VanCutsem, Geoffroy <geoffroy.vancutsem@...>; acrn-
dev@...
Cc: Chen, Gang C <gang.c.chen@...>; Zhang, Di <di.zhang@...>;
Jin, Zhi <zhi.jin@...>; Liu, Xiaojing <xiaojing.liu@...>
Subject: Re: [acrn-dev] [PATCH v2] tools:acrn-crashlog: Document of
configuration file
I send out the patch V3
On 08/10/2018 03:04 PM, VanCutsem, Geoffroy wrote:Just a couple of nitpicks below then you can move forward with my ACK.I'm sure David will have suggestions for you but these are best addressed
directly in the PR phase.
Corrected.you just confirm?
I'm assuming you have built that documentation locally and verified it, can
Patch v3 have been confirmed by local doc building system. I put the
"images/dot files" into conf.rst, since the doc building system collects only rst
files to doc directory.crash.dot
Thanks,
Geoffroy-----Original Message-----s/fields/fields
From: acrn-dev@...
[mailto:acrn-dev@...]
On Behalf Of Liu xinwu
Sent: Thursday, August 2, 2018 8:23 AM
To: acrn-dev@...
Cc: Chen, Gang C <gang.c.chen@...>; Zhang, Di
<di.zhang@...>; Liu, Xinwu <xinwu.liu@...>; Jin, Zhi
<zhi.jin@...>; Liu, Xiaojing <xiaojing.liu@...>;
VanCutsem, Geoffroy <geoffroy.vancutsem@...>
Subject: [acrn-dev] [PATCH v2] tools:acrn-crashlog: Document of
configuration file
This document explains all feilds in acrnprobe.xml.User could use it to control acrnprobe's behavior and configure their
own events.
Signed-off-by: Liu, Xinwu <xinwu.liu@...>
---
v1->v2:
1. refer conf.rst from README.rst
2. drawing by graphviz instead of raw text.
tools/acrn-crashlog/acrnprobe/README.rst | 2 +
tools/acrn-crashlog/acrnprobe/conf.rst | 337
+++++++++++++++++++++
.../acrn-crashlog/acrnprobe/images/build-crash.dot | 21 ++
.../acrn- crashlog/acrnprobe/images/crash-match.dot | 25 ++
4 files changed, 385 insertions(+)
create mode 100644 tools/acrn-crashlog/acrnprobe/conf.rst
create mode 100644 tools/acrn-crashlog/acrnprobe/images/build-conf`.create mode 100644 tools/acrn-crashlog/acrnprobe/images/crash-
match.dot
diff --git a/tools/acrn-crashlog/acrnprobe/README.rst b/tools/acrn-
crashlog/acrnprobe/README.rst index fa137f3..5f537ae 100644
--- a/tools/acrn-crashlog/acrnprobe/README.rst
+++ b/tools/acrn-crashlog/acrnprobe/README.rst
@@ -175,4 +175,6 @@ Configuration files
Custom configuration file that ``acrnprobe`` reads.
+More details about configuration file, please refer to :ref:`acrnprobe-ones.+
.. _`telemetrics-client`:
https://github.com/clearlinux/telemetrics-client
diff --git a/tools/acrn-crashlog/acrnprobe/conf.rst b/tools/acrn-
crashlog/acrnprobe/conf.rst new file mode 100644 index
0000000..7d8ec40
--- /dev/null
+++ b/tools/acrn-crashlog/acrnprobe/conf.rst
@@ -0,0 +1,337 @@
+.. _acrnprobe-conf:
+
+Configuration Of Acrnprobe
+##########################
+
+Description
+***********
+``Acrnprobe`` uses XML as the format of its configuration file,
+namely ``acrnprobe.xml``, so the configuration file should reach the
+`XML standard`_ at least.
+
+Layout
+******
+
+.. code-block:: xml
+
+ <?xml version="1.0" encoding="UTF-8"?>
+ <conf>
+ Root node of configuration.
+
+ <senders>
+ Configuration section of senders.
+ </senders>
+
+ <triggers>
+ Configuration section of triggers.
+ </triggers>
+
+ <vms>
+ Configuration section of virtual machines.
+ </vms>
+
+ <logs>
+ Configuration section of logs.
+ </logs>
+
+ <crashes>
+ Configuration section of crashes.
+ Note that this section should be configured after triggers and logs, as
+ crashes depend on these two sections.
+ </crashes>
+
+ <infos>
+ Configuration section of infos.
+ Note that this section should be configured after triggers and logs, as
+ infos depend on these two sections.
+ </infos>
+
+ </conf>
+
+As for the definition of ``sender``, ``trigger``, ``crash`` and
+``info`` and information of supported ``vm``, please refer to
:ref:`acrnprobe_doc`.
+
+Properties of modules
+*********************
+
+Common properties
+=================
+
+- ``id``:
+ The index, which grows from 1, in its group.
+- ``enable``:
+ This part of configuration can only take effect when the value is ``true``.
+
+Other properties
+================
+
+- ``inherit``:
+ Specify a parent for a certain crash.
+ The child crash will inherit all configurations from the specified
+(by id)
+ crash. These inherited configurations could be overwritten by newcrash B.+ Also, this property helps build the crash tree in ``acrnprobe``.
+- ``expression``:
+ See `Crash`_.
+
+Crash tree in acrnprobe
+***********************
+
+There could be a parent-child relationship between crashes. Refer to
+the diagrams below, crash B and D are the children of crash A,
+because crash B and D inherit from crash A, and crash C is the child ofgroup+"cannot" in one word.
+Build crash tree in configuration
+=================================
+
+.. graphviz:: images/build-crash.dot
+ :name: build-crash
+ :align: center
+ :caption: Build crash tree in configuration
+
+Crash type matches at runtime
+=============================
+
+In order to find a more specific type, if one crash type matches
+successfully ``acrnprobe`` will do match for each child of it (if it
+has) continually, and return the last successful one.
+Supposing these crash trees are like the diagram above at runtime:
+If a crash E is triggered, crash E will be returned immediately.
+If a crash A is triggered, then the candidates are crash A, B, C and D.
+The following diagram describes what ``acrnprobe`` will do if the
+matched result is crash D.
+
+.. graphviz:: images/crash-match.dot
+ :name: crash-match
+ :align: center
+ :caption: Crash type matches at runtime
+
+Sections
+********
+
+Sender
+======
+
+Example:
+
+.. code-block:: xml
+
+ <sender id="1" enable="true">
+ <name>crashlog</name>
+ <outdir>/var/log/crashlog</outdir>
+ <maxcrashdirs>1000</maxcrashdirs>
+ <maxlines>5000</maxlines>
+ <spacequota>90</spacequota>
+ <uptime>
+ <name>UPTIME</name>
+ <frequency>5</frequency>
+ <eventhours>6</eventhours>
+ </uptime>
+ </sender>
+
+* ``name``:
+ Name of sender. ``Acrnprobe`` uses this label to distinguish
+different
+ senders.
+ More information about sender, please refer to :ref:`acrnprobe_doc`.
+* ``outdir``:
+ Directory to store generated files of sender. ``Acrnprobe`` will
+create
+ this directory if it doesn't exist.
+* ``maxcrashdirs``:
+ The maximum serial number of generated ``crash directories``,
+``stat directories``
+ and ``vmevent directories``. The serial number will back to 0 if
+it reaches
+ the maximum. Only used by sender crashlog.
+* ``maxlines``:
+ The maximum lines of file ``history_event``. It will trigger
+action
+ ``"mv history_event history_event.bak"`` that the lines of
+``history_event``
+ reaches the ``maxlines``.
+* ``spacequota``:
+ ``Acrnprobe`` will stop collecting logs if it is true that
+ ``(used space / total space) * 100 > spacequota``. Only used by
+sender
+ crashlog.
+* ``uptime``:
+ Configuration to trigger ``UPTIME`` event.
+ sub-nodes:
+
+ + ``name``:
+ The name of event.
+ + ``frequency``:
+ Time interval in seconds to trigger ``uptime`` event.
+ + ``eventhours``:
+ Time interval in hours to generate a record.
+
+
+Trigger
+=======
+
+Example:
+
+.. code-block:: xml
+
+ <trigger id="1" enable="true">
+ <name>t_pstore</name>
+ <type>node</type>
+ <path>/sys/fs/pstore/console-ramoops-0</path>
+ </trigger>
+
+* ``name``:
+ The name of trigger. It's used by crash and info configuration module.
+* ``type`` and ``path``:
+ These two labels are used to get the content of trigger files.
+ ``Types`` have been implemented:
+
+ + ``node``:
+ ``Path`` can not support ``mmap(2)-like`` operations.
+ ``Acrnprobe`` can+ only use ``read(2)`` to get its content.
+ + ``file``:
+ ``Path`` is a regular file which supports ``mmap(2)-like`` operations.
+ + ``dir``:
+ ``Path`` is a directory.
+ + ``rebootreason``:
+ The reboot reason of system. The content of ``rebootreason`` is not
+ obtained in a common way. So, it doesn't work with ``path``.
+ + ``cmd``:
+ ``Path`` is a command which will be launched by ``execvp(3)``.
+
+ Some programs often use format ``string%d`` instead of static file
+ name to generate target file dynamically. So ``path`` supports
+ simple formats for these cases:
+
+ + /.../dir/string[*] --> all files with prefix "string" under dir.
+ + /.../dir/string[0] --> the first file of files, sorted by ``alphasort(3)``,
+ with prefix "string" under dir.
+ + /.../dir/string[-1] --> the last file of files, sorted by ``alphasort(3)``,
+ with prefix "string" under dir.
+
+Vm
+==
+
+Example:
+
+.. code-block:: xml
+
+ <vm id="1" enable="true">
+ <name>VM1</name>
+ <channel>polling</channel>
+ <interval>60</interval>
+ <syncevent id="1">CRASH/TOMBSTONE</syncevent>
+ <syncevent id="2">CRASH/UIWDT</syncevent>
+ <syncevent id="3">CRASH/IPANIC</syncevent>
+ <syncevent id="4">REBOOT</syncevent>
+ </vm>
+
+* ``name``:
+ The name of virtual machine.
+* ``channel``:
+ The ``channel`` name to get the virtual machine events.
+* ``interval``:
+ Time interval in seconds of polling vm's image.
+* ``syncevent``:
+ Event type ``acrnprobe`` will synchronize from virtual machine's
``crashlog``.
+ User could specify different types by id. The event type can also
+ be indicated by ``type/subtype``.
+
+Log
+===
+
+Example:
+
+.. code-block:: xml
+
+ <log id="1" enable="true">
+ <name>pstore</name>
+ <type>node</type>
+ <path>/sys/fs/pstore/console-ramoops-0</path>
+ </log>
+
+* ``name``:
+ By default, ``acrnprobe`` will take this ``name`` as generated
+log's name in
+ ``outdir`` of sender crashlog.
+ If ``path`` is specified by simple formats (includes [*], [0] or
+[-1]) the
+ file name of generated logs will be the same as original. More
+details about
+ simple formats, see `Trigger`_.
+* ``type`` and ``path``:
+ Same as `Trigger`_.
+* ``lines``:
+ By default, all contents in the original will be copied to generated log.
+ If this label is configured, only the ``lines`` at the end in the
+original
+ will be copied to the generated log. It takes effect only when the
+``type`` is
+ ``file``.
+
+Crash
+=====
+
+Example:
+
+.. code-block:: xml
+
+ <crash id='1' inherit='0' enable='true'>
+ <name>UNKNOWN</name>
+ <trigger>t_rebootreason</trigger>
+ <channel>oneshot</channel>
+ <content id='1'>WARM</content>
+ <log id='1'>pstore</log>
+ <log id='2'>acrnlog_last</log>
+ </crash>
+ <crash id='2' inherit='1' enable='true'>
+ <name>IPANIC</name>
+ <trigger>t_pstore</trigger>
+ <content id='1'> </content>
+ <mightcontent expression='1' id='1'>Kernel panic - not
syncing:</mightcontent>
+ <mightcontent expression='1' id='2'>BUG: unable to handle
kernel</mightcontent>
+ <data id='1'>kernel BUG at</data>
+ <data id='2'>EIP is at</data>
+ <data id='3'>Comm:</data>
+ </crash>
+
+* ``name``:
+ The type of the ``crash``.
+* ``trigger``:
+ The trigger name of the crash.
+* ``channel``:
+ The name of channel crash use.
+* ``content`` and ``mightcontent``:
+ They're used to match crash type. The match is successful if all
+the
+ following conditions are met:
+
+ a. All ``contents`` with different ``ids`` are included in trigger's
+ content.
+ b. One of ``mightcontents`` with the same ``expression`` is included in
+ trigger's content at least.
+ c. If there are ``mightcontents`` with different ``expressions``, eachmodule.+ with the same ``expression`` should meet condition b.
+* ``log``:
+ The log to be collected. The value is the configured ``name`` in logA"];+ User could specify different logs by ``id``.
+* ``data``:
+ It is used to generated ``DATA`` fields in ``crashfile``.
+``Acrnprobe`` will
+ copy the line which starts with configured ``data`` in trigger's
+content
+ to ``DATA`` fields. There are 3 fields in ``crashfile`` and they
+could be
+ specified by ``id`` 1, 2, 3.
+
+Info
+=====
+
+Example:
+
+.. code-block:: xml
+
+ <info id='1' enable='true'>
+ <name>BOOT_LOGS</name>
+ <trigger>t_boot</trigger>
+ <channel>oneshot</channel>
+ <log id='1'>kmsg</log>
+ <log id='2'>cmdline</log>
+ <log id='3'>acrnlog_cur</log>
+ <log id='4'>acrnlog_last</log>
+ </info>
+
+* ``name``:
+ The type of info.
+* ``trigger``:
+ The trigger name of the info.
+* ``channel``:
+ The name of channel info use.
+* ``log``:
+ The log to be collected. The value is the configured name in log
+module. User
+ could specify different logs by id.
+
+.. _`XML standard`: http://www.w3.org/TR/REC-xml
diff --git a/tools/acrn-crashlog/acrnprobe/images/build-crash.dot
b/tools/acrn-crashlog/acrnprobe/images/build-crash.dot
new file mode 100644
index 0000000..08e7628
--- /dev/null
+++ b/tools/acrn-crashlog/acrnprobe/images/build-crash.dot
@@ -0,0 +1,21 @@
+digraph {
+ {
+ node [shape=plaintext];
+ "level 1" -> "level 2" -> "level 3";
+ }
+
+ node [shape=box;style="rounded,filled";color=AntiqueWhite;];
+ c1 [ label="crash A\nid 1\ncrash root" ];
+ c2 [ label="crash B\nid 2" ];
+ c3 [ label="crash C\nid 3\ncrash leaf" ];
+ c4 [ label="crash D\nid 4\ncrash leaf" ];
+ c5 [ label="crash E\nid 5\ncrash root\ncrash leaf" ];
+ { rank = same; "level 1"; c1; c5;}
+ { rank = same; "level 2"; c2; c4;}
+ { rank = same; "level 3"; c3;}
+
+ node [shape=box;color="transparent";];
+ "None" -> {c1 c5} [ label="inherit 0" ];
+ c1 -> {c2 c4} [ label="inherit 1" ];
+ c2 -> c3 [ label="inherit 2" ];
+}
diff --git a/tools/acrn-crashlog/acrnprobe/images/crash-match.dot
b/tools/acrn-crashlog/acrnprobe/images/crash-match.dot
new file mode 100644
index 0000000..7da5ce0
--- /dev/null
+++ b/tools/acrn-crashlog/acrnprobe/images/crash-match.dot
@@ -0,0 +1,25 @@
+digraph {
+ {
+ node [shape=plaintext];
+ "level 1" -> "level 2" -> "level 3";
+ }
+
+ node [shape=box;style="rounded,filled";color=AntiqueWhite;];
+ c1 [ label="crash A\nid 1\ncrash root" ];
+ c2 [ label="crash B\nid 2" ];
+ c3 [ label="crash C\nid 3\ncrash leaf" ];
+ c4 [ label="crash D\nid 4\ncrash leaf" ];
+ { rank = same; "level 1"; c1;}
+ { rank = same; "level 2"; c2; c4;}
+ { rank = same; "level 3"; c3;}
+
+ node [shape=box;style="rounded,dashed";];
+ exp1 [ label="crash B matches fail\nmatch for the next child\nof crash+ exp2 [ label="crash D matches successfully\nreturn crash D"];
+
+ node [shape=box;style="invis";];
+ "channel" -> c1 [ label="trigger" ]
+ c1 -> {exp1 exp2}
+ exp1 -> c2 -> c3 [ style=dashed dir=none]
+ exp2 -> c4
+}
--
2.7.4
Re: [PATCH v3] tools:acrn-crashlog: Document of configuration file
toggle quoted message
Show quoted text
-----Original Message-----Acked-by: Geoffroy Van Cutsem <geoffroy.vancutsem@...>
From: Liu, Xinwu
Sent: Monday, August 13, 2018 4:37 AM
To: acrn-dev@...
Cc: Chen, Gang C <gang.c.chen@...>; Zhang, Di <di.zhang@...>;
Liu, Xinwu <xinwu.liu@...>; Jin, Zhi <zhi.jin@...>; Liu, Xiaojing
<xiaojing.liu@...>; VanCutsem, Geoffroy
<geoffroy.vancutsem@...>
Subject: [PATCH v3] tools:acrn-crashlog: Document of configuration file
This document explains all fields in acrnprobe.xml.
User could use it to control acrnprobe's behavior and configure their own
events.
Signed-off-by: Liu, Xinwu <xinwu.liu@...>
---
v1->v2:
1. refer conf.rst from README.rst
2. drawing by graphviz instead of raw text.
v2->v3:
1. correct some typo
2. put dot files into conf.rst, since the build system of doc collects only rst
files
tools/acrn-crashlog/acrnprobe/README.rst | 2 +
tools/acrn-crashlog/acrnprobe/conf.rst | 379
+++++++++++++++++++++++++++++++
2 files changed, 381 insertions(+)
create mode 100644 tools/acrn-crashlog/acrnprobe/conf.rst
diff --git a/tools/acrn-crashlog/acrnprobe/README.rst b/tools/acrn-
crashlog/acrnprobe/README.rst
index fa137f3..5f537ae 100644
--- a/tools/acrn-crashlog/acrnprobe/README.rst
+++ b/tools/acrn-crashlog/acrnprobe/README.rst
@@ -175,4 +175,6 @@ Configuration files
Custom configuration file that ``acrnprobe`` reads.
+More details about configuration file, please refer to :ref:`acrnprobe-conf`.
+
.. _`telemetrics-client`: https://github.com/clearlinux/telemetrics-client
diff --git a/tools/acrn-crashlog/acrnprobe/conf.rst b/tools/acrn-
crashlog/acrnprobe/conf.rst
new file mode 100644
index 0000000..f88d411
--- /dev/null
+++ b/tools/acrn-crashlog/acrnprobe/conf.rst
@@ -0,0 +1,379 @@
+.. _acrnprobe-conf:
+
+Configuration Of Acrnprobe
+##########################
+
+Description
+***********
+``Acrnprobe`` uses XML as the format of its configuration file, namely
+``acrnprobe.xml``, so the configuration file should reach the `XML
+standard`_ at least.
+
+Layout
+******
+
+.. code-block:: xml
+
+ <?xml version="1.0" encoding="UTF-8"?>
+ <conf>
+ Root node of configuration.
+
+ <senders>
+ Configuration section of senders.
+ </senders>
+
+ <triggers>
+ Configuration section of triggers.
+ </triggers>
+
+ <vms>
+ Configuration section of virtual machines.
+ </vms>
+
+ <logs>
+ Configuration section of logs.
+ </logs>
+
+ <crashes>
+ Configuration section of crashes.
+ Note that this section should be configured after triggers and logs, as
+ crashes depend on these two sections.
+ </crashes>
+
+ <infos>
+ Configuration section of infos.
+ Note that this section should be configured after triggers and logs, as
+ infos depend on these two sections.
+ </infos>
+
+ </conf>
+
+As for the definition of ``sender``, ``trigger``, ``crash`` and
+``info`` and information of supported ``vm``, please refer to
:ref:`acrnprobe_doc`.
+
+Properties of modules
+*********************
+
+Common properties
+=================
+
+- ``id``:
+ The index, which grows from 1, in its group.
+- ``enable``:
+ This part of configuration can only take effect when the value is ``true``.
+
+Other properties
+================
+
+- ``inherit``:
+ Specify a parent for a certain crash.
+ The child crash will inherit all configurations from the specified
+(by id)
+ crash. These inherited configurations could be overwritten by new ones.
+ Also, this property helps build the crash tree in ``acrnprobe``.
+- ``expression``:
+ See `Crash`_.
+
+Crash tree in acrnprobe
+***********************
+
+There could be a parent-child relationship between crashes. Refer to
+the diagrams below, crash B and D are the children of crash A, because
+crash B and D inherit from crash A, and crash C is the child of crash B.
+
+Build crash tree in configuration
+=================================
+
+.. graphviz::
+
+ digraph {
+ {
+ node [shape=plaintext];
+ "level 1" -> "level 2" -> "level 3";
+ }
+
+ node [shape=box;style="rounded,filled";color=AntiqueWhite;];
+ c1 [ label="crash A\nid 1\ncrash root" ];
+ c2 [ label="crash B\nid 2" ];
+ c3 [ label="crash C\nid 3\ncrash leaf" ];
+ c4 [ label="crash D\nid 4\ncrash leaf" ];
+ c5 [ label="crash E\nid 5\ncrash root\ncrash leaf" ];
+ { rank = same; "level 1"; c1; c5;}
+ { rank = same; "level 2"; c2; c4;}
+ { rank = same; "level 3"; c3;}
+
+ node [shape=box;color="transparent";];
+ "None" -> {c1 c5} [ label="inherit 0" ];
+ c1 -> {c2 c4} [ label="inherit 1" ];
+ c2 -> c3 [ label="inherit 2" ];
+ }
+
+Crash type matches at runtime
+=============================
+
+In order to find a more specific type, if one crash type matches
+successfully ``acrnprobe`` will do match for each child of it (if it
+has) continually, and return the last successful one.
+Supposing these crash trees are like the diagram above at runtime:
+If a crash E is triggered, crash E will be returned immediately.
+If a crash A is triggered, then the candidates are crash A, B, C and D.
+The following diagram describes what ``acrnprobe`` will do if the
+matched result is crash D.
+
+.. graphviz::
+
+ digraph {
+ {
+ node [shape=plaintext];
+ "level 1" -> "level 2" -> "level 3";
+ }
+
+ node [shape=box;style="rounded,filled";color=AntiqueWhite;];
+ c1 [ label="crash A\nid 1\ncrash root" ];
+ c2 [ label="crash B\nid 2" ];
+ c3 [ label="crash C\nid 3\ncrash leaf" ];
+ c4 [ label="crash D\nid 4\ncrash leaf" ];
+ { rank = same; "level 1"; c1;}
+ { rank = same; "level 2"; c2; c4;}
+ { rank = same; "level 3"; c3;}
+
+ node [shape=box;style="rounded,dashed";];
+ exp1 [ label="crash B matches fail\nmatch for the next child\nof crash
A"];
+ exp2 [ label="crash D matches successfully\nreturn crash D"];
+
+ node [shape=box;style="invis";];
+ "channel" -> c1 [ label="trigger" ]
+ c1 -> {exp1 exp2}
+ exp1 -> c2 -> c3 [ style=dashed dir=none]
+ exp2 -> c4
+ }
+
+Sections
+********
+
+Sender
+======
+
+Example:
+
+.. code-block:: xml
+
+ <sender id="1" enable="true">
+ <name>crashlog</name>
+ <outdir>/var/log/crashlog</outdir>
+ <maxcrashdirs>1000</maxcrashdirs>
+ <maxlines>5000</maxlines>
+ <spacequota>90</spacequota>
+ <uptime>
+ <name>UPTIME</name>
+ <frequency>5</frequency>
+ <eventhours>6</eventhours>
+ </uptime>
+ </sender>
+
+* ``name``:
+ Name of sender. ``Acrnprobe`` uses this label to distinguish
+different
+ senders.
+ More information about sender, please refer to :ref:`acrnprobe_doc`.
+* ``outdir``:
+ Directory to store generated files of sender. ``Acrnprobe`` will
+create
+ this directory if it doesn't exist.
+* ``maxcrashdirs``:
+ The maximum serial number of generated ``crash directories``, ``stat
+directories``
+ and ``vmevent directories``. The serial number will back to 0 if it
+reaches
+ the maximum. Only used by sender crashlog.
+* ``maxlines``:
+ The maximum lines of file ``history_event``. It will trigger action
+ ``"mv history_event history_event.bak"`` that the lines of
+``history_event``
+ reaches the ``maxlines``.
+* ``spacequota``:
+ ``Acrnprobe`` will stop collecting logs if it is true that
+ ``(used space / total space) * 100 > spacequota``. Only used by
+sender
+ crashlog.
+* ``uptime``:
+ Configuration to trigger ``UPTIME`` event.
+ sub-nodes:
+
+ + ``name``:
+ The name of event.
+ + ``frequency``:
+ Time interval in seconds to trigger ``uptime`` event.
+ + ``eventhours``:
+ Time interval in hours to generate a record.
+
+
+Trigger
+=======
+
+Example:
+
+.. code-block:: xml
+
+ <trigger id="1" enable="true">
+ <name>t_pstore</name>
+ <type>node</type>
+ <path>/sys/fs/pstore/console-ramoops-0</path>
+ </trigger>
+
+* ``name``:
+ The name of trigger. It's used by crash and info configuration module.
+* ``type`` and ``path``:
+ These two labels are used to get the content of trigger files.
+ ``Types`` have been implemented:
+
+ + ``node``:
+ ``Path`` cannot support ``mmap(2)-like`` operations. ``Acrnprobe`` can
+ only use ``read(2)`` to get its content.
+ + ``file``:
+ ``Path`` is a regular file which supports ``mmap(2)-like`` operations.
+ + ``dir``:
+ ``Path`` is a directory.
+ + ``rebootreason``:
+ The reboot reason of system. The content of ``rebootreason`` is not
+ obtained in a common way. So, it doesn't work with ``path``.
+ + ``cmd``:
+ ``Path`` is a command which will be launched by ``execvp(3)``.
+
+ Some programs often use format ``string%d`` instead of static file
+ name to generate target file dynamically. So ``path`` supports simple
+ formats for these cases:
+
+ + /.../dir/string[*] --> all files with prefix "string" under dir.
+ + /.../dir/string[0] --> the first file of files, sorted by ``alphasort(3)``,
+ with prefix "string" under dir.
+ + /.../dir/string[-1] --> the last file of files, sorted by ``alphasort(3)``,
+ with prefix "string" under dir.
+
+Vm
+==
+
+Example:
+
+.. code-block:: xml
+
+ <vm id="1" enable="true">
+ <name>VM1</name>
+ <channel>polling</channel>
+ <interval>60</interval>
+ <syncevent id="1">CRASH/TOMBSTONE</syncevent>
+ <syncevent id="2">CRASH/UIWDT</syncevent>
+ <syncevent id="3">CRASH/IPANIC</syncevent>
+ <syncevent id="4">REBOOT</syncevent>
+ </vm>
+
+* ``name``:
+ The name of virtual machine.
+* ``channel``:
+ The ``channel`` name to get the virtual machine events.
+* ``interval``:
+ Time interval in seconds of polling vm's image.
+* ``syncevent``:
+ Event type ``acrnprobe`` will synchronize from virtual machine's
``crashlog``.
+ User could specify different types by id. The event type can also be
+ indicated by ``type/subtype``.
+
+Log
+===
+
+Example:
+
+.. code-block:: xml
+
+ <log id="1" enable="true">
+ <name>pstore</name>
+ <type>node</type>
+ <path>/sys/fs/pstore/console-ramoops-0</path>
+ </log>
+
+* ``name``:
+ By default, ``acrnprobe`` will take this ``name`` as generated log's
+name in
+ ``outdir`` of sender crashlog.
+ If ``path`` is specified by simple formats (includes [*], [0] or
+[-1]) the
+ file name of generated logs will be the same as original. More
+details about
+ simple formats, see `Trigger`_.
+* ``type`` and ``path``:
+ Same as `Trigger`_.
+* ``lines``:
+ By default, all contents in the original will be copied to generated log.
+ If this label is configured, only the ``lines`` at the end in the
+original
+ will be copied to the generated log. It takes effect only when the
+``type`` is
+ ``file``.
+
+Crash
+=====
+
+Example:
+
+.. code-block:: xml
+
+ <crash id='1' inherit='0' enable='true'>
+ <name>UNKNOWN</name>
+ <trigger>t_rebootreason</trigger>
+ <channel>oneshot</channel>
+ <content id='1'>WARM</content>
+ <log id='1'>pstore</log>
+ <log id='2'>acrnlog_last</log>
+ </crash>
+ <crash id='2' inherit='1' enable='true'>
+ <name>IPANIC</name>
+ <trigger>t_pstore</trigger>
+ <content id='1'> </content>
+ <mightcontent expression='1' id='1'>Kernel panic - not
syncing:</mightcontent>
+ <mightcontent expression='1' id='2'>BUG: unable to handle
kernel</mightcontent>
+ <data id='1'>kernel BUG at</data>
+ <data id='2'>EIP is at</data>
+ <data id='3'>Comm:</data>
+ </crash>
+
+* ``name``:
+ The type of the ``crash``.
+* ``trigger``:
+ The trigger name of the crash.
+* ``channel``:
+ The name of channel crash use.
+* ``content`` and ``mightcontent``:
+ They're used to match crash type. The match is successful if all the
+ following conditions are met:
+
+ a. All ``contents`` with different ``ids`` are included in trigger's
+ content.
+ b. One of ``mightcontents`` with the same ``expression`` is included in
+ trigger's content at least.
+ c. If there are ``mightcontents`` with different ``expressions``, each group
+ with the same ``expression`` should meet condition b.
+* ``log``:
+ The log to be collected. The value is the configured ``name`` in log module.
+ User could specify different logs by ``id``.
+* ``data``:
+ It is used to generated ``DATA`` fields in ``crashfile``.
+``Acrnprobe`` will
+ copy the line which starts with configured ``data`` in trigger's
+content
+ to ``DATA`` fields. There are 3 fields in ``crashfile`` and they
+could be
+ specified by ``id`` 1, 2, 3.
+
+Info
+=====
+
+Example:
+
+.. code-block:: xml
+
+ <info id='1' enable='true'>
+ <name>BOOT_LOGS</name>
+ <trigger>t_boot</trigger>
+ <channel>oneshot</channel>
+ <log id='1'>kmsg</log>
+ <log id='2'>cmdline</log>
+ <log id='3'>acrnlog_cur</log>
+ <log id='4'>acrnlog_last</log>
+ </info>
+
+* ``name``:
+ The type of info.
+* ``trigger``:
+ The trigger name of the info.
+* ``channel``:
+ The name of channel info use.
+* ``log``:
+ The log to be collected. The value is the configured name in log
+module. User
+ could specify different logs by id.
+
+.. _`XML standard`: http://www.w3.org/TR/REC-xml
--
2.7.4
Re: [PATCH] HV: hv_main: Add #ifdef HV_DEBUG before debug specific code
#ifdef
On 08-13 Mon 10:01, Eddie Dong wrote:
In general we should do in different patch for bug fix. But this one is simple for those compile option --- this looks like MISRAC patches, we can do together to saveGot it.
-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Kaige Fu
Sent: Monday, August 13, 2018 5:18 PM
To: acrn-dev@...
Subject: Re: [acrn-dev] [PATCH] HV: hv_main: Add #ifdef HV_DEBUG before
debug specific code
On 08-13 Mon 08:27, Eddie Dong wrote:use?
On 08-13 Mon 07:59, Eddie Dong wrote:This is part of SBUF component, what kind of compile option do wevariables.Let us use same one.
shell command "vmexit" is the only one user of vmexit_time/cnt and
we use the get_vmexit_profile, enclosed by HV_DEBUG, to access theseAbsolutely, we should do it. This should be done in another patch, right?
Ok then.The sbuf has a hypercall, we need compile option to cover all sbuf code...BTW, hcall side also needs this kind of compile option.I can't follow here. Can you make it more clear.
Re: [PATCH] hv: treewide: fix Macro redefine, usage -- and operation violations
Junjie Mao
junjunshan1 <junjun.shan@...> writes:
1. Each line should be no more than 80 characters. Break the
paragraph if it is longer than that.
2. Due to the first rule, there typically is an empty line between
adjacent paragraphs/items.
With a minor style issue below.
Please also set your name properly by executing 'git config --global
user.name <your name>'.
operand per our coding style.
--
Best Regards
Junjie Mao
MISARC has requirements about Marco redefinition, usage of ++ or -- and assignment operator in boolean expression.Some general conventions about commit logs:
This patch is used to solve these violations.
The modifications are summarized as following:
1.The HC_VM_SET_MEMORY_REGION, HC_VM_GPA2HPA, HC_VM_SET_MEMORY_REGIONS are redefined twice in acrn_hv_des.h, so delete them to solve the macro redefinition violations.
2.The macro BUS_LOCK are redefined in bits.h and atomic.h, then delete the declaration in both two files, add a new declaration in cpu.h and include the header file.
3.modify the code to deprecate the usage of -- operators in string.c.
4.modify the while loop to for loop to avoid assignment operator in
boolean expression in vlapic.c.
1. Each line should be no more than 80 characters. Break the
paragraph if it is longer than that.
2. Due to the first rule, there typically is an empty line between
adjacent paragraphs/items.
Reviewed-by: Junjie Mao <junjie.mao@...>
Signed-off-by: junjunshan1 <junjun.shan@...>
With a minor style issue below.
Please also set your name properly by executing 'git config --global
user.name <your name>'.
---A single space is required between a binary operator and either of its
hypervisor/arch/x86/guest/vlapic.c | 3 ++-
hypervisor/include/arch/x86/cpu.h | 2 ++
hypervisor/include/lib/atomic.h | 3 +--
hypervisor/include/lib/bits.h | 3 +--
hypervisor/include/public/acrn_hv_defs.h | 3 ---
hypervisor/lib/string.c | 6 ++++--
6 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c
index 35be79c..72acb76 100644
--- a/hypervisor/arch/x86/guest/vlapic.c
+++ b/hypervisor/arch/x86/guest/vlapic.c
@@ -1135,7 +1135,8 @@ vlapic_icrlo_write_handler(struct acrn_vlapic *vlapic)
break;
}
- while ((vcpu_id = ffs64(dmask)) != INVALID_BIT_INDEX) {
+ for (vcpu_id = ffs64(dmask); vcpu_id != INVALID_BIT_INDEX;
+ vcpu_id = ffs64(dmask)) {
bitmap_clear_lock(vcpu_id, &dmask);
target_vcpu = vcpu_from_vid(vlapic->vm, vcpu_id);
if (target_vcpu == NULL) {
diff --git a/hypervisor/include/arch/x86/cpu.h b/hypervisor/include/arch/x86/cpu.h
index e56f443..d24228d 100644
--- a/hypervisor/include/arch/x86/cpu.h
+++ b/hypervisor/include/arch/x86/cpu.h
@@ -152,6 +152,8 @@
#ifndef ASSEMBLER
+#define BUS_LOCK "lock ; "
+
/**
*
* Identifiers for architecturally defined registers.
diff --git a/hypervisor/include/lib/atomic.h b/hypervisor/include/lib/atomic.h
index 264917c..6085863 100644
--- a/hypervisor/include/lib/atomic.h
+++ b/hypervisor/include/lib/atomic.h
@@ -29,8 +29,7 @@
#ifndef ATOMIC_H
#define ATOMIC_H
-
-#define BUS_LOCK "lock ; "
+#include <cpu.h>
#define build_atomic_load(name, size, type, ptr) \
static inline type name(const volatile type *ptr) \
diff --git a/hypervisor/include/lib/bits.h b/hypervisor/include/lib/bits.h
index 04a5057..ea0e8a1 100644
--- a/hypervisor/include/lib/bits.h
+++ b/hypervisor/include/lib/bits.h
@@ -29,8 +29,7 @@
#ifndef BITS_H
#define BITS_H
-
-#define BUS_LOCK "lock ; "
+#include <cpu.h>
/**
*
* INVALID_BIT_INDEX means when input paramter is zero,
diff --git a/hypervisor/include/public/acrn_hv_defs.h b/hypervisor/include/public/acrn_hv_defs.h
index cfeb834..700ce92 100644
--- a/hypervisor/include/public/acrn_hv_defs.h
+++ b/hypervisor/include/public/acrn_hv_defs.h
@@ -55,9 +55,6 @@
#define HC_VM_GPA2HPA BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x01UL)
#define HC_VM_SET_MEMORY_REGIONS BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x02UL)
#define HC_VM_WRITE_PROTECT_PAGE BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x03UL)
-#define HC_VM_SET_MEMORY_REGION BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x00UL)
-#define HC_VM_GPA2HPA BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x01UL)
-#define HC_VM_SET_MEMORY_REGIONS BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x02UL)
/* PCI assignment*/
#define HC_ID_PCI_BASE 0x50UL
diff --git a/hypervisor/lib/string.c b/hypervisor/lib/string.c
index db4f145..7d2a5a5 100644
--- a/hypervisor/lib/string.c
+++ b/hypervisor/lib/string.c
@@ -210,7 +210,8 @@ char *strcpy_s(char *d_arg, size_t dmax, const char *s_arg)
while (dest_avail > 0U) {
if (overlap_guard == 0U) {
pr_err("%s: overlap happened.", __func__);
- *(--d) = '\0';
+ d = d-1;
operand per our coding style.
+ *d = '\0';Ditto.
return NULL;
}
@@ -293,7 +294,8 @@ char *strncpy_s(char *d_arg, size_t dmax, const char *s_arg, size_t slen_arg)
while (dest_avail > 0U) {
if (overlap_guard == 0U) {
pr_err("%s: overlap happened.", __func__);
- *(--d) = '\0';
+ d = d-1;
--
Best Regards
Junjie Mao
+ *d = '\0';
return NULL;
}
[PATCH] hv: treewide: fix Macro redefine, usage -- and operation violations
Junjun Shan
MISARC has requirements about Marco redefinition, usage of ++ or -- and assignment operator in boolean expression.
This patch is used to solve these violations.
The modifications are summarized as following:
1.The HC_VM_SET_MEMORY_REGION, HC_VM_GPA2HPA, HC_VM_SET_MEMORY_REGIONS are redefined twice in acrn_hv_des.h, so delete them to solve the macro redefinition violations.
2.The macro BUS_LOCK are redefined in bits.h and atomic.h, then delete the declaration in both two files, add a new declaration in cpu.h and include the header file.
3.modify the code to deprecate the usage of -- operators in string.c.
4.modify the while loop to for loop to avoid assignment operator in boolean expression in vlapic.c.
Signed-off-by: junjunshan1 <junjun.shan@...>
---
hypervisor/arch/x86/guest/vlapic.c | 3 ++-
hypervisor/include/arch/x86/cpu.h | 2 ++
hypervisor/include/lib/atomic.h | 3 +--
hypervisor/include/lib/bits.h | 3 +--
hypervisor/include/public/acrn_hv_defs.h | 3 ---
hypervisor/lib/string.c | 6 ++++--
6 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c
index 35be79c..72acb76 100644
--- a/hypervisor/arch/x86/guest/vlapic.c
+++ b/hypervisor/arch/x86/guest/vlapic.c
@@ -1135,7 +1135,8 @@ vlapic_icrlo_write_handler(struct acrn_vlapic *vlapic)
break;
}
- while ((vcpu_id = ffs64(dmask)) != INVALID_BIT_INDEX) {
+ for (vcpu_id = ffs64(dmask); vcpu_id != INVALID_BIT_INDEX;
+ vcpu_id = ffs64(dmask)) {
bitmap_clear_lock(vcpu_id, &dmask);
target_vcpu = vcpu_from_vid(vlapic->vm, vcpu_id);
if (target_vcpu == NULL) {
diff --git a/hypervisor/include/arch/x86/cpu.h b/hypervisor/include/arch/x86/cpu.h
index e56f443..d24228d 100644
--- a/hypervisor/include/arch/x86/cpu.h
+++ b/hypervisor/include/arch/x86/cpu.h
@@ -152,6 +152,8 @@
#ifndef ASSEMBLER
+#define BUS_LOCK "lock ; "
+
/**
*
* Identifiers for architecturally defined registers.
diff --git a/hypervisor/include/lib/atomic.h b/hypervisor/include/lib/atomic.h
index 264917c..6085863 100644
--- a/hypervisor/include/lib/atomic.h
+++ b/hypervisor/include/lib/atomic.h
@@ -29,8 +29,7 @@
#ifndef ATOMIC_H
#define ATOMIC_H
-
-#define BUS_LOCK "lock ; "
+#include <cpu.h>
#define build_atomic_load(name, size, type, ptr) \
static inline type name(const volatile type *ptr) \
diff --git a/hypervisor/include/lib/bits.h b/hypervisor/include/lib/bits.h
index 04a5057..ea0e8a1 100644
--- a/hypervisor/include/lib/bits.h
+++ b/hypervisor/include/lib/bits.h
@@ -29,8 +29,7 @@
#ifndef BITS_H
#define BITS_H
-
-#define BUS_LOCK "lock ; "
+#include <cpu.h>
/**
*
* INVALID_BIT_INDEX means when input paramter is zero,
diff --git a/hypervisor/include/public/acrn_hv_defs.h b/hypervisor/include/public/acrn_hv_defs.h
index cfeb834..700ce92 100644
--- a/hypervisor/include/public/acrn_hv_defs.h
+++ b/hypervisor/include/public/acrn_hv_defs.h
@@ -55,9 +55,6 @@
#define HC_VM_GPA2HPA BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x01UL)
#define HC_VM_SET_MEMORY_REGIONS BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x02UL)
#define HC_VM_WRITE_PROTECT_PAGE BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x03UL)
-#define HC_VM_SET_MEMORY_REGION BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x00UL)
-#define HC_VM_GPA2HPA BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x01UL)
-#define HC_VM_SET_MEMORY_REGIONS BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x02UL)
/* PCI assignment*/
#define HC_ID_PCI_BASE 0x50UL
diff --git a/hypervisor/lib/string.c b/hypervisor/lib/string.c
index db4f145..7d2a5a5 100644
--- a/hypervisor/lib/string.c
+++ b/hypervisor/lib/string.c
@@ -210,7 +210,8 @@ char *strcpy_s(char *d_arg, size_t dmax, const char *s_arg)
while (dest_avail > 0U) {
if (overlap_guard == 0U) {
pr_err("%s: overlap happened.", __func__);
- *(--d) = '\0';
+ d = d-1;
+ *d = '\0';
return NULL;
}
@@ -293,7 +294,8 @@ char *strncpy_s(char *d_arg, size_t dmax, const char *s_arg, size_t slen_arg)
while (dest_avail > 0U) {
if (overlap_guard == 0U) {
pr_err("%s: overlap happened.", __func__);
- *(--d) = '\0';
+ d = d-1;
+ *d = '\0';
return NULL;
}
--
2.7.4
This patch is used to solve these violations.
The modifications are summarized as following:
1.The HC_VM_SET_MEMORY_REGION, HC_VM_GPA2HPA, HC_VM_SET_MEMORY_REGIONS are redefined twice in acrn_hv_des.h, so delete them to solve the macro redefinition violations.
2.The macro BUS_LOCK are redefined in bits.h and atomic.h, then delete the declaration in both two files, add a new declaration in cpu.h and include the header file.
3.modify the code to deprecate the usage of -- operators in string.c.
4.modify the while loop to for loop to avoid assignment operator in boolean expression in vlapic.c.
Signed-off-by: junjunshan1 <junjun.shan@...>
---
hypervisor/arch/x86/guest/vlapic.c | 3 ++-
hypervisor/include/arch/x86/cpu.h | 2 ++
hypervisor/include/lib/atomic.h | 3 +--
hypervisor/include/lib/bits.h | 3 +--
hypervisor/include/public/acrn_hv_defs.h | 3 ---
hypervisor/lib/string.c | 6 ++++--
6 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/hypervisor/arch/x86/guest/vlapic.c b/hypervisor/arch/x86/guest/vlapic.c
index 35be79c..72acb76 100644
--- a/hypervisor/arch/x86/guest/vlapic.c
+++ b/hypervisor/arch/x86/guest/vlapic.c
@@ -1135,7 +1135,8 @@ vlapic_icrlo_write_handler(struct acrn_vlapic *vlapic)
break;
}
- while ((vcpu_id = ffs64(dmask)) != INVALID_BIT_INDEX) {
+ for (vcpu_id = ffs64(dmask); vcpu_id != INVALID_BIT_INDEX;
+ vcpu_id = ffs64(dmask)) {
bitmap_clear_lock(vcpu_id, &dmask);
target_vcpu = vcpu_from_vid(vlapic->vm, vcpu_id);
if (target_vcpu == NULL) {
diff --git a/hypervisor/include/arch/x86/cpu.h b/hypervisor/include/arch/x86/cpu.h
index e56f443..d24228d 100644
--- a/hypervisor/include/arch/x86/cpu.h
+++ b/hypervisor/include/arch/x86/cpu.h
@@ -152,6 +152,8 @@
#ifndef ASSEMBLER
+#define BUS_LOCK "lock ; "
+
/**
*
* Identifiers for architecturally defined registers.
diff --git a/hypervisor/include/lib/atomic.h b/hypervisor/include/lib/atomic.h
index 264917c..6085863 100644
--- a/hypervisor/include/lib/atomic.h
+++ b/hypervisor/include/lib/atomic.h
@@ -29,8 +29,7 @@
#ifndef ATOMIC_H
#define ATOMIC_H
-
-#define BUS_LOCK "lock ; "
+#include <cpu.h>
#define build_atomic_load(name, size, type, ptr) \
static inline type name(const volatile type *ptr) \
diff --git a/hypervisor/include/lib/bits.h b/hypervisor/include/lib/bits.h
index 04a5057..ea0e8a1 100644
--- a/hypervisor/include/lib/bits.h
+++ b/hypervisor/include/lib/bits.h
@@ -29,8 +29,7 @@
#ifndef BITS_H
#define BITS_H
-
-#define BUS_LOCK "lock ; "
+#include <cpu.h>
/**
*
* INVALID_BIT_INDEX means when input paramter is zero,
diff --git a/hypervisor/include/public/acrn_hv_defs.h b/hypervisor/include/public/acrn_hv_defs.h
index cfeb834..700ce92 100644
--- a/hypervisor/include/public/acrn_hv_defs.h
+++ b/hypervisor/include/public/acrn_hv_defs.h
@@ -55,9 +55,6 @@
#define HC_VM_GPA2HPA BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x01UL)
#define HC_VM_SET_MEMORY_REGIONS BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x02UL)
#define HC_VM_WRITE_PROTECT_PAGE BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x03UL)
-#define HC_VM_SET_MEMORY_REGION BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x00UL)
-#define HC_VM_GPA2HPA BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x01UL)
-#define HC_VM_SET_MEMORY_REGIONS BASE_HC_ID(HC_ID, HC_ID_MEM_BASE + 0x02UL)
/* PCI assignment*/
#define HC_ID_PCI_BASE 0x50UL
diff --git a/hypervisor/lib/string.c b/hypervisor/lib/string.c
index db4f145..7d2a5a5 100644
--- a/hypervisor/lib/string.c
+++ b/hypervisor/lib/string.c
@@ -210,7 +210,8 @@ char *strcpy_s(char *d_arg, size_t dmax, const char *s_arg)
while (dest_avail > 0U) {
if (overlap_guard == 0U) {
pr_err("%s: overlap happened.", __func__);
- *(--d) = '\0';
+ d = d-1;
+ *d = '\0';
return NULL;
}
@@ -293,7 +294,8 @@ char *strncpy_s(char *d_arg, size_t dmax, const char *s_arg, size_t slen_arg)
while (dest_avail > 0U) {
if (overlap_guard == 0U) {
pr_err("%s: overlap happened.", __func__);
- *(--d) = '\0';
+ d = d-1;
+ *d = '\0';
return NULL;
}
--
2.7.4
Re: [PATCH v3] HV: fix "missing for discarded return value" for vm related api
Victor Sun
On 8/13/2018 5:58 PM, Eddie Dong wrote:
I will fix it in my next version.
Is it better to return a error code of "-EFAULT"?
Please refer his comments on patch V2.
BR,
Victor
OK, this is my fault. I am not aware the code here has been changed to prepare_vm() recently.-----Original Message-----Why you want to change from prepare_vm to prepare_vm0? This is obvious issue.
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Victor Sun
Sent: Friday, August 10, 2018 11:32 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH v3] HV: fix "missing for discarded return value"
for vm related api
- add handler if prepare_vm0() failed;
- remove assert in start_vm() and return -1 if failed;
changelog:
v2 -> v3: replace panic with pr_fatal and return directly
if prepare_vm0() failed;
v1 -> v2: panic if prepare_vm0() failed instead of reboot system;
Signed-off-by: Victor Sun <victor.sun@...>
---
hypervisor/arch/x86/cpu.c | 9 ++++++---
hypervisor/arch/x86/guest/vm.c | 7 ++++---
hypervisor/common/hypercall.c | 3 +--
3 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/hypervisor/arch/x86/cpu.c b/hypervisor/arch/x86/cpu.c index
02cbaec..6944592 100644
--- a/hypervisor/arch/x86/cpu.c
+++ b/hypervisor/arch/x86/cpu.c
@@ -563,9 +563,12 @@ static void bsp_boot_post(void)
exec_vmxon_instr(BOOT_CPU_ID);
- prepare_vm(BOOT_CPU_ID);
-
- default_idle();
+ if (prepare_vm0() == 0) {
I will fix it in my next version.
The purpose here is want to return a failure.+ default_idle();Why -1 ?
+ } else {
+ pr_fatal("Prepare VM0 failed!");
+ return;
+ }
/* Control should not come here */
cpu_dead(BOOT_CPU_ID);
diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index 09ceadb..09d9fc0 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -331,7 +331,9 @@ int start_vm(struct vm *vm)
/* Only start BSP (vid = 0) and let BSP start other APs */
vcpu = vcpu_from_vid(vm, 0U);
- ASSERT(vcpu != NULL, "vm%d, vcpu0", vm->vm_id);
+ if (vcpu == NULL) {
+ return -1;
Is it better to return a error code of "-EFAULT"?
Anthony also has such concern. I already checked with Junjie on this and Junjie confirmed it is safe.+ }I remember FuSa wants to split them into 2 lines of instructions.
schedule_vcpu(vcpu);
return 0;
@@ -358,8 +360,7 @@ int reset_vm(struct vm *vm)
vioapic_reset(vm->arch_vm.virt_ioapic);
- start_vm(vm);
- return 0;
+ return (start_vm(vm));
Please refer his comments on patch V2.
BR,
Victor
}
/**
diff --git a/hypervisor/common/hypercall.c
b/hypervisor/common/hypercall.c index f8efb35..7489b99 100644
--- a/hypervisor/common/hypercall.c
+++ b/hypervisor/common/hypercall.c
@@ -280,8 +280,7 @@ int32_t hcall_reset_vm(uint16_t vmid)
if ((target_vm == NULL) || is_vm0(target_vm))
return -1;
- reset_vm(target_vm);
- return 0;
+ return (reset_vm(target_vm));
}
int32_t hcall_assert_irqline(struct vm *vm, uint16_t vmid, uint64_t param)
--
2.7.4
Re: [PATCH v2 1/3] HV: Add the emulation of CPUID with 0x16 leaf
Zhao, Yakui
toggle quoted message
Show quoted text
-----Original Message-----OK. I will remove them.
From: acrn-dev@... [mailto:acrn-dev@...]
On Behalf Of Eddie Dong
Sent: Monday, August 13, 2018 4:04 PM
To: acrn-dev@...; Xu, Anthony <anthony.xu@...>
Subject: Re: [acrn-dev] [PATCH v2 1/3] HV: Add the emulation of CPUID with
0x16 leafIf it is physical out of ranger, returning what hardware returns is better thanHi Yakui,guest_cpuid().
ACRN doesn't emulate CPUID >=0x80000000U at all.
So for CPUID >=0x80000000U, ACRN just read from hardware ininit_vcpuid_entry doesn't need to handle CPUID >=0x80000000U.After Eddie's suggestion is followed to add the limit check(>=0x15
CPUID), the mentioned check is not needed any more.
I keep them so that the init_vcpuid_entry still can return the zero
entry for the out-of-range CPUID input.
If you think that the check is redundant, I can remove them.
fixed-zero.
We want the code to be slim to save FuSa effort.
Thx Eddie
Re: [PATCH] HV: hv_main: Add #ifdef HV_DEBUG before debug specific code
#ifdef
In general we should do in different patch for bug fix. But this one is simple for those compile option --- this looks like MISRAC patches, we can do together to save
toggle quoted message
Show quoted text
-----Original Message-----
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Kaige Fu
Sent: Monday, August 13, 2018 5:18 PM
To: acrn-dev@...
Subject: Re: [acrn-dev] [PATCH] HV: hv_main: Add #ifdef HV_DEBUG before
debug specific code
On 08-13 Mon 08:27, Eddie Dong wrote:use?
On 08-13 Mon 07:59, Eddie Dong wrote:This is part of SBUF component, what kind of compile option do wevariables.Let us use same one.
shell command "vmexit" is the only one user of vmexit_time/cnt and
we use the get_vmexit_profile, enclosed by HV_DEBUG, to access theseAbsolutely, we should do it. This should be done in another patch, right?
Ok then.The sbuf has a hypercall, we need compile option to cover all sbuf code...BTW, hcall side also needs this kind of compile option.I can't follow here. Can you make it more clear.
Re: [PATCH v3] HV: fix "missing for discarded return value" for vm related api
toggle quoted message
Show quoted text
-----Original Message-----Why you want to change from prepare_vm to prepare_vm0? This is obvious issue.
From: acrn-dev@...
[mailto:acrn-dev@...] On Behalf Of Victor Sun
Sent: Friday, August 10, 2018 11:32 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH v3] HV: fix "missing for discarded return value"
for vm related api
- add handler if prepare_vm0() failed;
- remove assert in start_vm() and return -1 if failed;
changelog:
v2 -> v3: replace panic with pr_fatal and return directly
if prepare_vm0() failed;
v1 -> v2: panic if prepare_vm0() failed instead of reboot system;
Signed-off-by: Victor Sun <victor.sun@...>
---
hypervisor/arch/x86/cpu.c | 9 ++++++---
hypervisor/arch/x86/guest/vm.c | 7 ++++---
hypervisor/common/hypercall.c | 3 +--
3 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/hypervisor/arch/x86/cpu.c b/hypervisor/arch/x86/cpu.c index
02cbaec..6944592 100644
--- a/hypervisor/arch/x86/cpu.c
+++ b/hypervisor/arch/x86/cpu.c
@@ -563,9 +563,12 @@ static void bsp_boot_post(void)
exec_vmxon_instr(BOOT_CPU_ID);
- prepare_vm(BOOT_CPU_ID);
-
- default_idle();
+ if (prepare_vm0() == 0) {
+ default_idle();Why -1 ?
+ } else {
+ pr_fatal("Prepare VM0 failed!");
+ return;
+ }
/* Control should not come here */
cpu_dead(BOOT_CPU_ID);
diff --git a/hypervisor/arch/x86/guest/vm.c
b/hypervisor/arch/x86/guest/vm.c index 09ceadb..09d9fc0 100644
--- a/hypervisor/arch/x86/guest/vm.c
+++ b/hypervisor/arch/x86/guest/vm.c
@@ -331,7 +331,9 @@ int start_vm(struct vm *vm)
/* Only start BSP (vid = 0) and let BSP start other APs */
vcpu = vcpu_from_vid(vm, 0U);
- ASSERT(vcpu != NULL, "vm%d, vcpu0", vm->vm_id);
+ if (vcpu == NULL) {
+ return -1;
+ }I remember FuSa wants to split them into 2 lines of instructions.
schedule_vcpu(vcpu);
return 0;
@@ -358,8 +360,7 @@ int reset_vm(struct vm *vm)
vioapic_reset(vm->arch_vm.virt_ioapic);
- start_vm(vm);
- return 0;
+ return (start_vm(vm));
}
/**
diff --git a/hypervisor/common/hypercall.c
b/hypervisor/common/hypercall.c index f8efb35..7489b99 100644
--- a/hypervisor/common/hypercall.c
+++ b/hypervisor/common/hypercall.c
@@ -280,8 +280,7 @@ int32_t hcall_reset_vm(uint16_t vmid)
if ((target_vm == NULL) || is_vm0(target_vm))
return -1;
- reset_vm(target_vm);
- return 0;
+ return (reset_vm(target_vm));
}
int32_t hcall_assert_irqline(struct vm *vm, uint16_t vmid, uint64_t param)
--
2.7.4
Re: [PATCH] HV: hv_main: Add #ifdef HV_DEBUG before debug specific code
#ifdef
On 08-13 Mon 08:27, Eddie Dong wrote:
Absolutely, we should do it. This should be done in another patch, right?Ok then.
On 08-13 Mon 07:59, Eddie Dong wrote:This is part of SBUF component, what kind of compile option do we use?Let us use same one.
shell command "vmexit" is the only one user of vmexit_time/cnt and we use
the get_vmexit_profile, enclosed by HV_DEBUG, to access these variables.The sbuf has a hypercall, we need compile option to cover all sbuf code...BTW, hcall side also needs this kind of compile option.I can't follow here. Can you make it more clear.
Re: [PATCH 1/6] DM USB: xHCI: fix an xHCI issue to enable UOS s3 feature
Yu Wang
Acked-by: Yu Wang <yu1.wang@...>
toggle quoted message
Show quoted text
On 18-08-13 16:48:30, Wu, Xiaoguang wrote:
Current DM design use two variables to do the indexing of xHCI
Event Ring: er_enq_idx and er_events_cnt. They are members of
the struct pci_xhci_rtsregs.
In UOS, during the process of xHCI resuming, the xHCI driver
will restore the ERSTBA (Event Ring Segment Table Base Address)
register to be the value before suspending. And at this point,
the old DM implementation will set both er_enq_idx and
er_events_cnt to be zero, so the DM will access the Event Ring
from the start position in the buffer. But at the same time the
UOS xHCI driver still wants to access the old position in the
Event Ring before suspending, which will result of unexpected
errors.
Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index da0c42a..792d930 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -2732,9 +2732,6 @@ pci_xhci_rtsregs_write(struct pci_xhci_vdev *xdev,
rts->erst_p = XHCI_GADDR(xdev,
xdev->rtsregs.erstba_p->qwEvrsTablePtr & ~0x3FUL);
- rts->er_enq_idx = 0;
- rts->er_events_cnt = 0;
-
UPRINTF(LDBG, "wr erstba erst (%p) ptr 0x%lx, sz %u\r\n",
rts->erstba_p,
rts->erstba_p->qwEvrsTablePtr,
--
2.7.4
Re: [PATCH v3 1/1] vbs: fix virtio_vq_index_get func handling of multi VQ concurrent request.
Yu Wang
On 18-08-13 00:25:12, Ong Hock Yu wrote:
Under multiple VQ use case, it is possible to have concurrent requests.Haven't fix the v2 comment... Please add one log followed v2 suggestion
Added support to return multiple vq index from all vcpu.
Signed-off-by: Ong Hock Yu <ong.hock.yu@...>
---
drivers/vbs/vbs.c | 14 ++++++++------
drivers/vbs/vbs_rng.c | 2 +-
include/linux/vbs/vbs.h | 8 ++++++--
3 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/vbs/vbs.c b/drivers/vbs/vbs.c
index 65b8a0bbe3d3..a2f263cb91c0 100644
--- a/drivers/vbs/vbs.c
+++ b/drivers/vbs/vbs.c
@@ -148,9 +148,11 @@ long virtio_dev_deregister(struct virtio_dev_info *dev)
return 0;
}
-int virtio_vq_index_get(struct virtio_dev_info *dev, unsigned long *ioreqs_map)
+int virtio_vqs_index_get(struct virtio_dev_info *dev,
+ unsigned long *ioreqs_map,
+ int *vqs_index, int max_vqs_index)
{
- int val = -1;
+ int idx = 0;
struct vhm_request *req;
int vcpu;
@@ -159,7 +161,7 @@ int virtio_vq_index_get(struct virtio_dev_info *dev, unsigned long *ioreqs_map)
return -EINVAL;
}
- while (1) {
+ while (idx < max_vqs_index) {
vcpu = find_first_bit(ioreqs_map, dev->_ctx.max_vcpu);
if (vcpu == dev->_ctx.max_vcpu)
break;
@@ -179,9 +181,9 @@ int virtio_vq_index_get(struct virtio_dev_info *dev, unsigned long *ioreqs_map)
pr_debug("%s: write request! type %d\n",
__func__, req->type);
if (dev->io_range_type == PIO_RANGE)
- val = req->reqs.pio_request.value;
+ vqs_index[idx++] = req->reqs.pio_request.value;
else
- val = req->reqs.mmio_request.value;
+ vqs_index[idx++] = req->reqs.mmio_request.value;
}
smp_mb();
atomic_set(&req->processed, REQ_STATE_COMPLETE);
@@ -189,7 +191,7 @@ int virtio_vq_index_get(struct virtio_dev_info *dev, unsigned long *ioreqs_map)
}
}
- return val;
+ return idx;
}
static long virtio_vqs_info_set(struct virtio_dev_info *dev,
diff --git a/drivers/vbs/vbs_rng.c b/drivers/vbs/vbs_rng.c
index fd2bb27af66e..c5e28cc12c55 100644
--- a/drivers/vbs/vbs_rng.c
+++ b/drivers/vbs/vbs_rng.c
@@ -268,7 +268,7 @@ static int handle_kick(int client_id, unsigned long *ioreqs_map)
return -EINVAL;
}
- val = virtio_vq_index_get(&rng->dev, ioreqs_map);
+ virtio_vqs_index_get(&rng->dev, ioreqs_map, &val, 1);
if (val >= 0)
handle_vq_kick(rng, val);
diff --git a/include/linux/vbs/vbs.h b/include/linux/vbs/vbs.h
index 30df8ebf68a0..964bacac865c 100644
--- a/include/linux/vbs/vbs.h
+++ b/include/linux/vbs/vbs.h
@@ -262,7 +262,7 @@ long virtio_dev_register(struct virtio_dev_info *dev);
long virtio_dev_deregister(struct virtio_dev_info *dev);
/**
- * virtio_vq_index_get - get virtqueue index that frontend kicks
+ * virtio_vqs_index_get - get virtqueue indexes that frontend kicks
*
* This API is normally called in the VBS-K device's callback
* function, to get value write to the "kick" register from
@@ -270,10 +270,14 @@ long virtio_dev_deregister(struct virtio_dev_info *dev);
*
* @dev: Pointer to VBS-K device data struct
* @ioreqs_map: requests bitmap need to handle, provided by VHM
+ * @vqs_index: array to store the vq indexes
+ * @max_vqs_index: size of vqs_index array
*
* Return: >=0 on virtqueue index, <0 on error
*/
-int virtio_vq_index_get(struct virtio_dev_info *dev, unsigned long *ioreqs_map);
+int virtio_vqs_index_get(struct virtio_dev_info *dev,
+ unsigned long *ioreqs_map,
+ int *vqs_index, int max_vqs_index);
/**
* virtio_dev_reset - reset a VBS-K device
--
2.17.0
Re: [PATCH] tools: acrntrace: Add ring buffer mode
Can you please also modify the documentation (README.rst [1]) to reflect these changes?
[1] https://github.com/projectacrn/acrn-hypervisor/blob/master/tools/acrntrace/README.rst
Thanks,
Geoffroy
toggle quoted message
Show quoted text
[1] https://github.com/projectacrn/acrn-hypervisor/blob/master/tools/acrntrace/README.rst
Thanks,
Geoffroy
-----Original Message-----
From: acrn-dev@... [mailto:acrn-dev@...]
On Behalf Of Zhipeng Gong
Sent: Monday, August 13, 2018 10:20 AM
To: acrn-dev@...
Subject: [acrn-dev] [PATCH] tools: acrntrace: Add ring buffer mode
When running longevity test and capture acrntrace, generated acrntrace files
sizes are too big.
Sometimes we don't care very old trace. This patch adds ring buffer mode,
fixes acrntrace file size and overwrites old trace with new trace.
Signed-off-by: Zhipeng Gong <zhipeng.gong@...>
---
tools/acrntrace/acrntrace.c | 44
+++++++++++++++++++++++++++++++++++++++++---
tools/acrntrace/acrntrace.h | 1 +
2 files changed, 42 insertions(+), 3 deletions(-)
diff --git a/tools/acrntrace/acrntrace.c b/tools/acrntrace/acrntrace.c index
39fda35..371ffaa 100644
--- a/tools/acrntrace/acrntrace.c
+++ b/tools/acrntrace/acrntrace.c
@@ -37,16 +37,19 @@ static char trace_file_dir[TRACE_FILE_DIR_LEN];
static reader_struct *reader;
static int pcpu_num = 0;
+static int ring_buffer_size = 0;
static void display_usage(void)
{
printf("acrntrace - tool to collect ACRN trace data\n"
- "[Usage] acrntrace [-i] [period in msec] [-ch]\n\n"
+ "[Usage] acrntrace [-i] [period in msec] [-r] [ring buffer size] [-
ch]\n\n"
"[Options]\n"
"\t-h: print this message\n"
"\t-i: period_in_ms: specify polling interval [1-999]\n"
"\t-t: max time to capture trace data (in second)\n"
- "\t-c: clear the buffered old data\n");
+ "\t-c: clear the buffered old data\n"
+ "\t-r: ring_buffer_size (in MB): enable ring buffer mode,"
+ " where new trace overwrites old trace when full\n");
}
static void timer_handler(union sigval sv) @@ -113,6 +116,15 @@ static int
parse_opt(int argc, char *argv[])
timeout = ret;
pr_dbg("Capture trace data for at most %ds\n", ret);
break;
+ case 'r':
+ ret = atoi(optarg);
+ if (ret <= 0) {
+ pr_err("'-r' require integer greater than 0\n");
+ return -EINVAL;
+ }
+ ring_buffer_size = ret * 1024 * 1024;
+ pr_dbg("Ring buffer size is %dM\n", ret);
+ break;
case 'c':
flags |= FLAG_CLEAR_BUF;
break;
@@ -206,6 +218,15 @@ static void reader_fn(param_t * param)
trace_ev_t e;
struct statvfs stat;
uint64_t freespace;
+ int pos = 0;
+
+ if (ring_buffer_size != 0) {
+ param->buffer = malloc(ring_buffer_size);
+ if (!param->buffer) {
+ perror("Failed to allocate ring buffer\n");
+ return;
+ }
+ }
pr_dbg("reader thread[%lu] created for FILE*[0x%p]\n",
pthread_self(), fp);
@@ -219,7 +240,13 @@ static void reader_fn(param_t * param)
while (1) {
do {
- ret = sbuf_write(fd, sbuf);
+ if (ring_buffer_size != 0) {
+ ret = sbuf_get(sbuf, param->buffer + pos);
+ pos += sbuf->ele_size;
+ if (pos + sbuf->ele_size > ring_buffer_size)
+ pos = 0;
+ } else
+ ret = sbuf_write(fd, sbuf);
} while (ret > 0);
usleep(period);
@@ -284,6 +311,17 @@ static void destory_reader(reader_struct * reader)
reader->thrd = 0;
}
+ if (ring_buffer_size != 0 && reader->param.buffer) {
+ int ret;
+
+ ret = write(reader->param.trace_fd, reader->param.buffer,
+ ring_buffer_size);
+ if (ret != ring_buffer_size) {
+ perror("Failed to write ring buffer\n");
+ }
+ free(reader->param.buffer);
+ }
+
if (reader->param.sbuf) {
munmap(reader->param.sbuf, MMAP_SIZE);
reader->param.sbuf = NULL;
diff --git a/tools/acrntrace/acrntrace.h b/tools/acrntrace/acrntrace.h index
ea54f4d..6615157 100644
--- a/tools/acrntrace/acrntrace.h
+++ b/tools/acrntrace/acrntrace.h
@@ -76,6 +76,7 @@ typedef struct {
int trace_fd;
shared_buf_t *sbuf;
pthread_mutex_t *sbuf_lock;
+ uint8_t *buffer;
} param_t;
typedef struct {
--
2.7.4
Re: [PATCH 1/2] vhm: add ioeventfd support for ACRN hypervisor service module
Jie Deng
On 2018/8/13 14:40, Shuo Liu wrote:
+Is it better to be "(p->wildcard || p->data == data)" ?
+static struct acrn_vhm_ioeventfd *vhm_ioeventfd_match(
+ struct vhm_ioeventfd_info *info,
+ u64 addr, u64 data, int length, int type)
+{
+ struct acrn_vhm_ioeventfd *p = NULL;
+
+ list_for_each_entry(p, &info->ioeventfds, list) {
+ if (p->type == type &&
+ p->addr == addr &&
+ p->length == length &&
+ (p->data == data || p->wildcard))
+ return p;
[PATCH 4/6] DM USB: xHCI: refine port assignment logic
Wu, Xiaoguang
The variable native_assign_ports in struct pci_xhci_vdev is used
to record wether certain root hub port in SOS is assgned to UOS.
The logic is zero reserve and nonzero assignement.
In this patch, the meaning of native_assign_ports is expand to the
following logic:
native_assign_ports[x][y] = 0: native port x-y is not assigned.
native_assign_ports[x][y] < 0: native port x-y is assigned but
currently no virtual device attached.
native_assign_ports[x][y] > 0: native paort x-y is assigned and
virtual device attached to virtual port whose number
is stored in native_assign_ports[x][y].
Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index f12126b..2df7ce3 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -373,7 +373,16 @@ struct pci_xhci_vdev {
int (*excap_write)(struct pci_xhci_vdev *, uint64_t, uint64_t);
int usb2_port_start;
int usb3_port_start;
- uint8_t *native_assign_ports[USB_NATIVE_NUM_BUS];
+
+ /* native_assign_ports is used to record the assigment information:
+ * native_assign_ports[x][y] = 0: native port x-y is not assigned.
+ * native_assign_ports[x][y] < 0: native port x-y is assigned but
+ * currently no virtual device attached.
+ * native_assign_ports[x][y] > 0: native paort x-y is assigned and
+ * virtual device attached to virtual port whose number
+ * is stored in native_assign_ports[x][y].
+ */
+ int8_t *native_assign_ports[USB_NATIVE_NUM_BUS];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};
@@ -3365,14 +3374,14 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
if (!xdev->native_assign_ports[bus]) {
xdev->native_assign_ports[bus] = calloc(USB_NATIVE_NUM_PORT,
- sizeof(uint8_t));
+ sizeof(int8_t));
if (!xdev->native_assign_ports[bus]) {
rc = -3;
goto errout;
}
}
+ xdev->native_assign_ports[bus][port] = -1;
- xdev->native_assign_ports[bus][port] = 1;
errout:
if (rc)
UPRINTF(LWRN, "%s fails, rc=%d\r\n", __func__, rc);
--
2.7.4
to record wether certain root hub port in SOS is assgned to UOS.
The logic is zero reserve and nonzero assignement.
In this patch, the meaning of native_assign_ports is expand to the
following logic:
native_assign_ports[x][y] = 0: native port x-y is not assigned.
native_assign_ports[x][y] < 0: native port x-y is assigned but
currently no virtual device attached.
native_assign_ports[x][y] > 0: native paort x-y is assigned and
virtual device attached to virtual port whose number
is stored in native_assign_ports[x][y].
Signed-off-by: Xiaoguang Wu <xiaoguang.wu@...>
Reviewed-by: Liang Yang <liang3.yang@...>
---
devicemodel/hw/pci/xhci.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/devicemodel/hw/pci/xhci.c b/devicemodel/hw/pci/xhci.c
index f12126b..2df7ce3 100644
--- a/devicemodel/hw/pci/xhci.c
+++ b/devicemodel/hw/pci/xhci.c
@@ -373,7 +373,16 @@ struct pci_xhci_vdev {
int (*excap_write)(struct pci_xhci_vdev *, uint64_t, uint64_t);
int usb2_port_start;
int usb3_port_start;
- uint8_t *native_assign_ports[USB_NATIVE_NUM_BUS];
+
+ /* native_assign_ports is used to record the assigment information:
+ * native_assign_ports[x][y] = 0: native port x-y is not assigned.
+ * native_assign_ports[x][y] < 0: native port x-y is assigned but
+ * currently no virtual device attached.
+ * native_assign_ports[x][y] > 0: native paort x-y is assigned and
+ * virtual device attached to virtual port whose number
+ * is stored in native_assign_ports[x][y].
+ */
+ int8_t *native_assign_ports[USB_NATIVE_NUM_BUS];
struct timespec mf_prev_time; /* previous time of accessing MFINDEX */
};
@@ -3365,14 +3374,14 @@ pci_xhci_parse_bus_port(struct pci_xhci_vdev *xdev, char *opts)
if (!xdev->native_assign_ports[bus]) {
xdev->native_assign_ports[bus] = calloc(USB_NATIVE_NUM_PORT,
- sizeof(uint8_t));
+ sizeof(int8_t));
if (!xdev->native_assign_ports[bus]) {
rc = -3;
goto errout;
}
}
+ xdev->native_assign_ports[bus][port] = -1;
- xdev->native_assign_ports[bus][port] = 1;
errout:
if (rc)
UPRINTF(LWRN, "%s fails, rc=%d\r\n", __func__, rc);
--
2.7.4