* [Qemu-devel] [PATCH v8 0/2] Implement Hex file loader and add test case
@ 2018-05-15 1:45 Su Hang
2018-05-15 1:45 ` [Qemu-devel] [PATCH v8 1/2] Implement .hex file loader Su Hang
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Su Hang @ 2018-05-15 1:45 UTC (permalink / raw)
To: stefanha, jim, joel; +Cc: qemu-devel
These series of patchs implement Intel Hexadecimal File loader and
add QTest testcase to verify the correctness of Loader.
v1:
-- Basic version.
v2:
-- Replace `do{}while(cond);` block with `for(;;)` block.
v3:
-- Add two new files information in MAINTAINERS.
v4:
-- Correct the 'test.hex' path in hexloader-test.c.
v5:
-- Split huge parse_hex_blob() function into four small function;
-- Add check for memory bounds;
-- Check validation for Record type;
-- Replace function ctoh() with glib function g_ascii_xdigit_value();
-- Remove check for '.hex' suffix;
-- Remove unnecessary type cast;
-- Remove duplicate zero-initialization;
-- Correct typos;
v6:
-- Call Intel HEX loader after load_uimage_as();
-- Remove use of KERNEL_LOAD_ADDR;
-- Change (hwaddr) type argument to (hwaddr *) type;
-- Use new struct HexParser to contain all arguments needed by
handle_record_type();
-- Enable discontiguous data records (again);
-- Remove unnecessary memory clean: bin_buf and HexLine line;
-- Correct typo;
-- Remove unnecessary check for hex file size;
-- Add entry point handle for START_SEG_ADDR_RECORD and
START_LINEAR_ADDR_RECORD record type;
-- Use hwaddr * type to store entry point;
v7:
-- Remove unnecessary code style changes;
-- Replace `bool` with `size_t` for function `parse_record` return type;
-- Replace `int` with `size_t` for struct `HexParser` member field
`total_size` type;
-- Replace `int` with `size_t` for function `handle_record_type` return type;
-- Return an error when encountered unimplemented data type
`START_SEG_ADDR_RECORD`;
-- Add check for `START_LINEAR_ADDR_RECORD` data type:
byte_count == 4 and address == 0;
-- Rename struct `HexParser` member field `addr` to `start_addr`;
-- Replace `int` with `size_t` for function `parse_hex_blob` return type;
-- Stop incrementing `record_index` when encountered whitespace;
-- Replace
`if ((record_index >> 1) - LEN_EXCEPT_DATA != parser.line.byte_count)`
with
`if ((LEN_EXCEPT_DATA + parser.line.byte_count) * 2 != record_index)`
-- Remove unused field `max_sz`;
-- Replace `int` with `size_t` for local variable `total_size`
in function `parse_hex_blob`
-- Rename function `load_image_targphys_hex_as` argument `addr` to `entry`;
v8:
-- Discard change in code logic: Stop attempting load 32-bit kernel after
failed to load 64-bit kernel image;
-- Rename function name `load_image_targphys_hex_as` to `load_targphys_hex_as`
in comment;
-- Correct grammatical errors in comments;
-- Drop non-exist argument description in comment;
-- Drop macro function `rom_add_hex_blob_as`, replace it with
`rom_add_blob_fixed_as`, because they are equivalent;
Su Hang (2):
Implement .hex file loader
Add QTest testcase for the Intel Hexadecimal Object File Loader.
MAINTAINERS | 6 +
hw/arm/boot.c | 7 +-
hw/core/loader.c | 246 +++++++++++++++++++++++++++++++++++
include/hw/loader.h | 12 ++
tests/Makefile.include | 2 +
tests/hex-loader-check-data/test.hex | 12 ++
tests/hexloader-test.c | 56 ++++++++
7 files changed, 340 insertions(+), 1 deletion(-)
create mode 100644 tests/hex-loader-check-data/test.hex
create mode 100644 tests/hexloader-test.c
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v8 1/2] Implement .hex file loader
2018-05-15 1:45 [Qemu-devel] [PATCH v8 0/2] Implement Hex file loader and add test case Su Hang
@ 2018-05-15 1:45 ` Su Hang
2018-05-15 10:04 ` Stefan Hajnoczi
2018-05-15 1:45 ` [Qemu-devel] [PATCH v8 2/2] Add QTest testcase for the Intel Hexadecimal Object File Loader Su Hang
2018-05-15 10:07 ` [Qemu-devel] [PATCH v8 0/2] Implement Hex file loader and add test case Stefan Hajnoczi
2 siblings, 1 reply; 7+ messages in thread
From: Su Hang @ 2018-05-15 1:45 UTC (permalink / raw)
To: stefanha, jim, joel; +Cc: qemu-devel
This patch adds Intel Hexadecimal Object File format support to
the loader. The file format specification is available here:
http://www.piclist.com/techref/fileext/hex/intel.htm
The file format is mainly intended for embedded systems
and microcontrollers, such as Micro:bit Arduino, ARM, STM32, etc.
Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
---
hw/arm/boot.c | 7 +-
hw/core/loader.c | 246 ++++++++++++++++++++++++++++++++++++++++++++++++++++
include/hw/loader.h | 12 +++
3 files changed, 264 insertions(+), 1 deletion(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 1e2be20731e5..da254a6646e6 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -1066,12 +1066,17 @@ static void arm_load_kernel_notify(Notifier *notifier, void *data)
kernel_size = load_uimage_as(info->kernel_filename, &entry, NULL,
&is_linux, NULL, NULL, as);
}
+ if (kernel_size < 0) {
+ /* 32-bit ARM Intel HEX file */
+ entry = 0;
+ kernel_size = load_targphys_hex_as(info->kernel_filename, &entry, as);
+ }
if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) && kernel_size < 0) {
kernel_size = load_aarch64_image(info->kernel_filename,
info->loader_start, &entry, as);
is_linux = 1;
} else if (kernel_size < 0) {
- /* 32-bit ARM */
+ /* 32-bit ARM binary file */
entry = info->loader_start + KERNEL_LOAD_ADDR;
kernel_size = load_image_targphys_as(info->kernel_filename, entry,
info->ram_size - KERNEL_LOAD_ADDR,
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 06bdbca53709..3c0202caa88f 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1286,3 +1286,249 @@ void hmp_info_roms(Monitor *mon, const QDict *qdict)
}
}
}
+
+typedef enum HexRecord HexRecord;
+enum HexRecord {
+ DATA_RECORD = 0,
+ EOF_RECORD,
+ EXT_SEG_ADDR_RECORD,
+ START_SEG_ADDR_RECORD,
+ EXT_LINEAR_ADDR_RECORD,
+ START_LINEAR_ADDR_RECORD,
+};
+
+#define DATA_FIELD_MAX_LEN 0xff
+#define LEN_EXCEPT_DATA 0x5
+/* 0x5 = sizeof(byte_count) + sizeof(address) + sizeof(record_type) +
+ * sizeof(checksum) */
+typedef struct {
+ uint8_t byte_count;
+ uint16_t address;
+ uint8_t record_type;
+ uint8_t data[DATA_FIELD_MAX_LEN];
+ uint8_t checksum;
+} HexLine;
+
+/* return 0 or -1 if error */
+static bool parse_record(HexLine *line, uint8_t *our_checksum, const uint8_t c,
+ uint32_t *index, const bool in_process)
+{
+ /* +-------+---------------+-------+---------------------+--------+
+ * | byte | |record | | |
+ * | count | address | type | data |checksum|
+ * +-------+---------------+-------+---------------------+--------+
+ * ^ ^ ^ ^ ^ ^
+ * |1 byte | 2 bytes |1 byte | 0-255 bytes | 1 byte |
+ */
+ uint8_t value = 0;
+ uint32_t idx = *index;
+ /* ignore space */
+ if (g_ascii_isspace(c)) {
+ return true;
+ }
+ if (!g_ascii_isxdigit(c) || !in_process) {
+ return false;
+ }
+ value = g_ascii_xdigit_value(c);
+ value = idx & 0x1 ? value & 0xf : value << 4;
+ if (idx < 2) {
+ line->byte_count |= value;
+ } else if (2 <= idx && idx < 6) {
+ line->address <<= 4;
+ line->address += g_ascii_xdigit_value(c);
+ } else if (6 <= idx && idx < 8) {
+ line->record_type |= value;
+ } else if (8 <= idx && idx < 8 + 2 * line->byte_count) {
+ line->data[(idx - 8) >> 1] |= value;
+ } else if (8 + 2 * line->byte_count <= idx &&
+ idx < 10 + 2 * line->byte_count) {
+ line->checksum |= value;
+ } else {
+ return false;
+ }
+ *our_checksum += value;
+ ++(*index);
+ return true;
+}
+
+typedef struct {
+ const char *filename;
+ HexLine line;
+ uint8_t *bin_buf;
+ hwaddr *start_addr;
+ int total_size;
+ uint32_t next_address_to_write;
+ uint32_t current_address;
+ uint32_t current_rom_index;
+ uint32_t rom_start_address;
+ AddressSpace *as;
+} HexParser;
+
+/* return size or -1 if error */
+static int handle_record_type(HexParser *parser)
+{
+ HexLine *line = &(parser->line);
+ switch (line->record_type) {
+ case DATA_RECORD:
+ parser->current_address =
+ (parser->next_address_to_write & 0xffff0000) | line->address;
+ /* verify this is a contiguous block of memory */
+ if (parser->current_address != parser->next_address_to_write) {
+ if (parser->current_rom_index != 0) {
+ rom_add_blob_fixed_as(parser->filename, parser->bin_buf,
+ parser->current_rom_index,
+ parser->rom_start_address, parser->as);
+ }
+ parser->rom_start_address = parser->current_address;
+ parser->current_rom_index = 0;
+ }
+
+ /* copy from line buffer to output bin_buf */
+ memcpy(parser->bin_buf + parser->current_rom_index, line->data,
+ line->byte_count);
+ parser->current_rom_index += line->byte_count;
+ parser->total_size += line->byte_count;
+ /* save next address to write */
+ parser->next_address_to_write =
+ parser->current_address + line->byte_count;
+ break;
+
+ case EOF_RECORD:
+ if (parser->current_rom_index != 0) {
+ rom_add_blob_fixed_as(parser->filename, parser->bin_buf,
+ parser->current_rom_index,
+ parser->rom_start_address, parser->as);
+ }
+ return parser->total_size;
+ case EXT_SEG_ADDR_RECORD:
+ case EXT_LINEAR_ADDR_RECORD:
+ if (line->byte_count != 2 && line->address != 0) {
+ return -1;
+ }
+
+ if (parser->current_rom_index != 0) {
+ rom_add_blob_fixed_as(parser->filename, parser->bin_buf,
+ parser->current_rom_index,
+ parser->rom_start_address, parser->as);
+ }
+
+ /* save next address to write,
+ * in case of non-contiguous block of memory */
+ parser->next_address_to_write =
+ line->record_type == EXT_SEG_ADDR_RECORD
+ ? ((line->data[0] << 12) | (line->data[1] << 4))
+ : ((line->data[0] << 24) | (line->data[1] << 16));
+ parser->rom_start_address = parser->next_address_to_write;
+ parser->current_rom_index = 0;
+ break;
+
+ case START_SEG_ADDR_RECORD:
+ /* TODO: START_SEG_ADDR_RECORD is x86-specific */
+ return -1;
+
+ case START_LINEAR_ADDR_RECORD:
+ if (line->byte_count != 4 && line->address != 0) {
+ return -1;
+ }
+
+ *(parser->start_addr) = (line->data[0] << 24) | (line->data[1] << 16) |
+ (line->data[2] << 8) | (line->data[3]);
+ break;
+
+ default:
+ return -1;
+ }
+
+ return parser->total_size;
+}
+
+/* return size or -1 if error */
+static int parse_hex_blob(const char *filename, hwaddr *addr, uint8_t *hex_blob,
+ off_t hex_blob_size, uint8_t *bin_buf,
+ AddressSpace *as)
+{
+ bool in_process = false; /* avoid re-enter and
+ * check whether record begin with ':' */
+ uint8_t *end = hex_blob + hex_blob_size;
+ uint8_t our_checksum = 0;
+ uint32_t record_index = 0;
+ HexParser parser = {0};
+ parser.filename = filename;
+ parser.bin_buf = bin_buf;
+ parser.start_addr = addr;
+ parser.as = as;
+
+ for (; hex_blob < end; ++hex_blob) {
+ switch (*hex_blob) {
+ case '\r':
+ case '\n':
+ if (!in_process) {
+ break;
+ }
+
+ in_process = false;
+ if ((LEN_EXCEPT_DATA + parser.line.byte_count) * 2 !=
+ record_index ||
+ our_checksum != 0) {
+ return -1;
+ }
+
+ if (handle_record_type(&parser) == -1) {
+ return -1;
+ }
+ break;
+
+ /* start of a new record. */
+ case ':':
+ memset(&parser.line, 0, sizeof(HexLine));
+ in_process = true;
+ record_index = 0;
+ break;
+
+ /* decoding lines */
+ default:
+ if (!parse_record(&parser.line, &our_checksum, *hex_blob,
+ &record_index, in_process)) {
+ return -1;
+ }
+ break;
+ }
+ }
+ return parser.total_size;
+}
+
+/* return size or -1 if error */
+int load_targphys_hex_as(const char *filename, hwaddr *entry, AddressSpace *as)
+{
+ off_t hex_blob_size;
+ uint8_t *hex_blob;
+ uint8_t *bin_buf;
+ int fd;
+ int total_size = 0;
+
+ fd = open(filename, O_RDONLY);
+ if (fd < 0) {
+ return -1;
+ }
+ hex_blob_size = lseek(fd, 0, SEEK_END);
+ if (hex_blob_size < 0) {
+ close(fd);
+ return -1;
+ }
+ hex_blob = g_malloc(hex_blob_size);
+ bin_buf = g_malloc(hex_blob_size);
+ lseek(fd, 0, SEEK_SET);
+ if (read(fd, hex_blob, hex_blob_size) != hex_blob_size) {
+ total_size = -1;
+ goto hex_parser_exit;
+ }
+
+ total_size =
+ parse_hex_blob(filename, entry, hex_blob, hex_blob_size, bin_buf, as);
+
+hex_parser_exit:
+ close(fd);
+ g_free(hex_blob);
+ g_free(bin_buf);
+ return total_size;
+}
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 5ed3fd8ae67a..bd9d7b380076 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -28,6 +28,18 @@ ssize_t load_image_size(const char *filename, void *addr, size_t size);
int load_image_targphys_as(const char *filename,
hwaddr addr, uint64_t max_sz, AddressSpace *as);
+/**load_targphys_hex_as:
+ * @filename: Path to the .hex file
+ * @entry: Store the entry point given by the .hex file
+ * @as: The AddressSpace to load the .hex file to. The value of
+ * address_space_memory is used if nothing is supplied here.
+ *
+ * Load a fixed .hex file into memory.
+ *
+ * Returns the size of the loaded .hex file on success, -1 otherwise.
+ */
+int load_targphys_hex_as(const char *filename, hwaddr *entry, AddressSpace *as);
+
/** load_image_targphys:
* Same as load_image_targphys_as(), but doesn't allow the caller to specify
* an AddressSpace.
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v8 2/2] Add QTest testcase for the Intel Hexadecimal Object File Loader.
2018-05-15 1:45 [Qemu-devel] [PATCH v8 0/2] Implement Hex file loader and add test case Su Hang
2018-05-15 1:45 ` [Qemu-devel] [PATCH v8 1/2] Implement .hex file loader Su Hang
@ 2018-05-15 1:45 ` Su Hang
2018-05-15 10:05 ` Stefan Hajnoczi
2018-05-15 10:07 ` [Qemu-devel] [PATCH v8 0/2] Implement Hex file loader and add test case Stefan Hajnoczi
2 siblings, 1 reply; 7+ messages in thread
From: Su Hang @ 2018-05-15 1:45 UTC (permalink / raw)
To: stefanha, jim, joel; +Cc: qemu-devel
'test.hex' file is a bare metal ARM software stored in Hexadecimal
Object Format. When it's loaded by QEMU, it will print "Hello world!\n"
on console.
`pre_store` array in 'hexloader-test.c' file, stores the binary format
of 'test.hex' file, which is used to verify correctness.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
---
MAINTAINERS | 6 ++++
tests/Makefile.include | 2 ++
tests/hex-loader-check-data/test.hex | 12 ++++++++
tests/hexloader-test.c | 56 ++++++++++++++++++++++++++++++++++++
4 files changed, 76 insertions(+)
create mode 100644 tests/hex-loader-check-data/test.hex
create mode 100644 tests/hexloader-test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 459e3594e16c..c86095e3b3d9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1291,6 +1291,12 @@ F: hw/core/generic-loader.c
F: include/hw/core/generic-loader.h
F: docs/generic-loader.txt
+Intel Hexadecimal Object File Loader
+M: Su Hang <suhang16@mails.ucas.ac.cn>
+S: Maintained
+F: tests/hexloader-test.c
+F: tests/hex-loader-check-data/test.hex
+
CHRP NVRAM
M: Thomas Huth <thuth@redhat.com>
S: Maintained
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 3b9a5e31a2c2..f4a3e71f34ee 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -380,6 +380,7 @@ check-qtest-arm-y += tests/test-arm-mptimer$(EXESUF)
gcov-files-arm-y += hw/timer/arm_mptimer.c
check-qtest-arm-y += tests/boot-serial-test$(EXESUF)
check-qtest-arm-y += tests/sdhci-test$(EXESUF)
+check-qtest-arm-y += tests/hexloader-test$(EXESUF)
check-qtest-aarch64-y = tests/numa-test$(EXESUF)
check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
@@ -755,6 +756,7 @@ tests/qmp-test$(EXESUF): tests/qmp-test.o
tests/device-introspect-test$(EXESUF): tests/device-introspect-test.o
tests/rtc-test$(EXESUF): tests/rtc-test.o
tests/m48t59-test$(EXESUF): tests/m48t59-test.o
+tests/hexloader-test$(EXESUF): tests/hexloader-test.o
tests/endianness-test$(EXESUF): tests/endianness-test.o
tests/spapr-phb-test$(EXESUF): tests/spapr-phb-test.o $(libqos-obj-y)
tests/prom-env-test$(EXESUF): tests/prom-env-test.o $(libqos-obj-y)
diff --git a/tests/hex-loader-check-data/test.hex b/tests/hex-loader-check-data/test.hex
new file mode 100644
index 000000000000..7e99b452f5cc
--- /dev/null
+++ b/tests/hex-loader-check-data/test.hex
@@ -0,0 +1,12 @@
+:020000040001F9
+:1000000004D09FE5160000EBFEFFFFEA9810010008
+:1000100004B02DE500B08DE20CD04DE208000BE5F8
+:10002000060000EA08301BE50020D3E52C309FE5F0
+:10003000002083E508301BE5013083E208300BE542
+:1000400008301BE50030D3E5000053E3F4FFFF1A4E
+:100050000000A0E100D08BE204B09DE41EFF2FE180
+:1000600000101F1000482DE904B08DE208009FE544
+:10007000E6FFFFEB0000A0E10088BDE8840001007E
+:1000800000101F1048656C6C6F20776F726C6421D4
+:020090000A0064
+:00000001FF
diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
new file mode 100644
index 000000000000..70f99ac03c6b
--- /dev/null
+++ b/tests/hexloader-test.c
@@ -0,0 +1,56 @@
+/*
+ * QTest testcase for the Intel Hexadecimal Object File Loader
+ *
+ * Authors:
+ * Su Hang <suhang16@mails.ucas.ac.cn> 2018
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+
+#define BIN_SIZE 146
+
+static unsigned char pre_store[BIN_SIZE] = {
+ 4, 208, 159, 229, 22, 0, 0, 235, 254, 255, 255, 234, 152, 16, 1,
+ 0, 4, 176, 45, 229, 0, 176, 141, 226, 12, 208, 77, 226, 8, 0,
+ 11, 229, 6, 0, 0, 234, 8, 48, 27, 229, 0, 32, 211, 229, 44,
+ 48, 159, 229, 0, 32, 131, 229, 8, 48, 27, 229, 1, 48, 131, 226,
+ 8, 48, 11, 229, 8, 48, 27, 229, 0, 48, 211, 229, 0, 0, 83,
+ 227, 244, 255, 255, 26, 0, 0, 160, 225, 0, 208, 139, 226, 4, 176,
+ 157, 228, 30, 255, 47, 225, 0, 16, 31, 16, 0, 72, 45, 233, 4,
+ 176, 141, 226, 8, 0, 159, 229, 230, 255, 255, 235, 0, 0, 160, 225,
+ 0, 136, 189, 232, 132, 0, 1, 0, 0, 16, 31, 16, 72, 101, 108,
+ 108, 111, 32, 119, 111, 114, 108, 100, 33, 10, 0};
+
+/* success if no crash or abort */
+static void hex_loader_test(void)
+{
+ unsigned int i;
+ unsigned char memory_content[BIN_SIZE];
+ const unsigned int base_addr = 0x00010000;
+
+ QTestState *s = qtest_startf(
+ "-M versatilepb -m 128M -nographic -kernel ../tests/hex-loader-check-data/test.hex");
+
+ for (i = 0; i < BIN_SIZE; ++i) {
+ memory_content[i] = qtest_readb(s, base_addr + i);
+ g_assert_cmpuint(memory_content[i], ==, pre_store[i]);
+ }
+ qtest_quit(s);
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+ g_test_init(&argc, &argv, NULL);
+
+ qtest_add_func("/tmp/hex_loader", hex_loader_test);
+ ret = g_test_run();
+
+ return ret;
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v8 1/2] Implement .hex file loader
2018-05-15 1:45 ` [Qemu-devel] [PATCH v8 1/2] Implement .hex file loader Su Hang
@ 2018-05-15 10:04 ` Stefan Hajnoczi
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2018-05-15 10:04 UTC (permalink / raw)
To: Su Hang; +Cc: jim, joel, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 756 bytes --]
On Tue, May 15, 2018 at 09:45:57AM +0800, Su Hang wrote:
> This patch adds Intel Hexadecimal Object File format support to
> the loader. The file format specification is available here:
> http://www.piclist.com/techref/fileext/hex/intel.htm
>
> The file format is mainly intended for embedded systems
> and microcontrollers, such as Micro:bit Arduino, ARM, STM32, etc.
>
> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
> ---
> hw/arm/boot.c | 7 +-
> hw/core/loader.c | 246 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/loader.h | 12 +++
> 3 files changed, 264 insertions(+), 1 deletion(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v8 2/2] Add QTest testcase for the Intel Hexadecimal Object File Loader.
2018-05-15 1:45 ` [Qemu-devel] [PATCH v8 2/2] Add QTest testcase for the Intel Hexadecimal Object File Loader Su Hang
@ 2018-05-15 10:05 ` Stefan Hajnoczi
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2018-05-15 10:05 UTC (permalink / raw)
To: Su Hang; +Cc: stefanha, jim, joel, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]
On Tue, May 15, 2018 at 09:45:58AM +0800, Su Hang wrote:
> 'test.hex' file is a bare metal ARM software stored in Hexadecimal
> Object Format. When it's loaded by QEMU, it will print "Hello world!\n"
> on console.
>
> `pre_store` array in 'hexloader-test.c' file, stores the binary format
> of 'test.hex' file, which is used to verify correctness.
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Su Hang <suhang16@mails.ucas.ac.cn>
> ---
> MAINTAINERS | 6 ++++
> tests/Makefile.include | 2 ++
> tests/hex-loader-check-data/test.hex | 12 ++++++++
> tests/hexloader-test.c | 56 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 76 insertions(+)
> create mode 100644 tests/hex-loader-check-data/test.hex
> create mode 100644 tests/hexloader-test.c
Please add the ./configure change needed to make this test work for
out-of-tree builds. I sent an emailing explaining how the necessary
symlink can be created in ./configure in reply to the previous series.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v8 0/2] Implement Hex file loader and add test case
2018-05-15 1:45 [Qemu-devel] [PATCH v8 0/2] Implement Hex file loader and add test case Su Hang
2018-05-15 1:45 ` [Qemu-devel] [PATCH v8 1/2] Implement .hex file loader Su Hang
2018-05-15 1:45 ` [Qemu-devel] [PATCH v8 2/2] Add QTest testcase for the Intel Hexadecimal Object File Loader Su Hang
@ 2018-05-15 10:07 ` Stefan Hajnoczi
2 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2018-05-15 10:07 UTC (permalink / raw)
To: Su Hang; +Cc: stefanha, jim, joel, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 304 bytes --]
On Tue, May 15, 2018 at 09:45:56AM +0800, Su Hang wrote:
> These series of patchs implement Intel Hexadecimal File loader and
> add QTest testcase to verify the correctness of Loader.
Good job, this looks close now. I left a comment on the test case - it
needs to work with out-of-tree builds.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v8 0/2] Implement Hex file loader and add test case
@ 2018-05-11 5:53 Su Hang
0 siblings, 0 replies; 7+ messages in thread
From: Su Hang @ 2018-05-11 5:53 UTC (permalink / raw)
To: stefanha, jim, joel; +Cc: qemu-devel
These series of patchs implement Intel Hexadecimal File loader and
add QTest testcase to verify the correctness of Loader.
v1:
-- Basic version.
v2:
-- Replace `do{}while(cond);` block with `for(;;)` block.
v3:
-- Add two new files information in MAINTAINERS.
v4:
-- Correct the 'test.hex' path in hexloader-test.c.
v5:
-- Split huge parse_hex_blob() function into four small function;
-- Add check for memory bounds;
-- Check validation for Record type;
-- Replace function ctoh() with glib function g_ascii_xdigit_value();
-- Remove check for '.hex' suffix;
-- Remove unnecessary type cast;
-- Remove duplicate zero-initialization;
-- Correct typos;
v6:
-- Call Intel HEX loader after load_uimage_as();
-- Remove use of KERNEL_LOAD_ADDR;
-- Change (hwaddr) type argument to (hwaddr *) type;
-- Use new struct HexParser to contain all arguments needed by
handle_record_type();
-- Enable discontiguous data records (again);
-- Remove unnecessary memory clean: bin_buf and HexLine line;
-- Correct typo;
-- Remove unnecessary check for hex file size;
-- Add entry point handle for START_SEG_ADDR_RECORD and
START_LINEAR_ADDR_RECORD record type;
-- Use hwaddr * type to store entry point;
v7:
-- Remove unnecessary code style changes;
-- Replace `bool` with `size_t` for function `parse_record` return type;
-- Replace `int` with `size_t` for struct `HexParser` member field
`total_size` type;
-- Replace `int` with `size_t` for function `handle_record_type` return type;
-- Return an error when encountered unimplemented data type
`START_SEG_ADDR_RECORD`;
-- Add check for `START_LINEAR_ADDR_RECORD` data type:
byte_count == 4 and address == 0;
-- Rename struct `HexParser` member field `addr` to `start_addr`;
-- Replace `int` with `size_t` for function `parse_hex_blob` return type;
-- Stop incrementing `record_index` when encountered whitespace;
-- Replace
`if ((record_index >> 1) - LEN_EXCEPT_DATA != parser.line.byte_count)`
with
`if ((LEN_EXCEPT_DATA + parser.line.byte_count) * 2 != record_index)`
-- Remove unused field `max_sz`;
-- Replace `int` with `size_t` for local variable `total_size`
in function `parse_hex_blob`
-- Rename function `load_image_targphys_hex_as` argument `addr` to `entry`;
v8:
-- Discard change in code logic: Stop attempting load 32-bit kernel after
failed to load 64-bit kernel image;
-- Rename function name `load_image_targphys_hex_as` to `load_targphys_hex_as`
in comment;
-- Correct grammatical errors in comments;
-- Drop non-exist argument description in comment;
-- Drop macro function `rom_add_hex_blob_as`, replace it with
`rom_add_blob_fixed_as`, because they are equivalent;
Su Hang (2):
Implement .hex file loader
Add QTest testcase for the Intel Hexadecimal Object File Loader.
MAINTAINERS | 6 +
hw/arm/boot.c | 7 +-
hw/core/loader.c | 246 +++++++++++++++++++++++++++++++++++
include/hw/loader.h | 12 ++
tests/Makefile.include | 2 +
tests/hex-loader-check-data/test.hex | 12 ++
tests/hexloader-test.c | 56 ++++++++
7 files changed, 340 insertions(+), 1 deletion(-)
create mode 100644 tests/hex-loader-check-data/test.hex
create mode 100644 tests/hexloader-test.c
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-05-15 10:07 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-15 1:45 [Qemu-devel] [PATCH v8 0/2] Implement Hex file loader and add test case Su Hang
2018-05-15 1:45 ` [Qemu-devel] [PATCH v8 1/2] Implement .hex file loader Su Hang
2018-05-15 10:04 ` Stefan Hajnoczi
2018-05-15 1:45 ` [Qemu-devel] [PATCH v8 2/2] Add QTest testcase for the Intel Hexadecimal Object File Loader Su Hang
2018-05-15 10:05 ` Stefan Hajnoczi
2018-05-15 10:07 ` [Qemu-devel] [PATCH v8 0/2] Implement Hex file loader and add test case Stefan Hajnoczi
-- strict thread matches above, loose matches on Subject: below --
2018-05-11 5:53 Su Hang
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.