* [PATCH v6 1/3] libqos/fw_cfg: refactor file directory iteraton to make it more reusable
2025-01-20 4:38 [PATCH v6 0/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files Ani Sinha
@ 2025-01-20 4:38 ` Ani Sinha
2025-01-20 4:38 ` [PATCH v6 2/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files Ani Sinha
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Ani Sinha @ 2025-01-20 4:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Gerd Hoffmann, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini
Cc: Ani Sinha, qemu-devel
fw-cfg file directory iteration code can be used by other functions that may
want to implement fw-cfg file operations. Refactor it into a smaller helper
so that it can be reused.
No functional change.
Signed-off-by: Ani Sinha <anisinha@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
---
tests/qtest/libqos/fw_cfg.c | 63 +++++++++++++++++++++++--------------
1 file changed, 40 insertions(+), 23 deletions(-)
diff --git a/tests/qtest/libqos/fw_cfg.c b/tests/qtest/libqos/fw_cfg.c
index 89f053ccac..22435873d9 100644
--- a/tests/qtest/libqos/fw_cfg.c
+++ b/tests/qtest/libqos/fw_cfg.c
@@ -60,6 +60,37 @@ static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
qtest_writew(fw_cfg->qts, fw_cfg->base, key);
}
+static bool find_pdir_entry(QFWCFG *fw_cfg, const char *filename,
+ uint16_t *sel, uint32_t *size)
+{
+ g_autofree unsigned char *filesbuf = NULL;
+ uint32_t count;
+ size_t dsize;
+ FWCfgFile *pdir_entry;
+ uint32_t i;
+ bool found = false;
+
+ *size = 0;
+ *sel = 0;
+
+ qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, &count, sizeof(count));
+ count = be32_to_cpu(count);
+ dsize = sizeof(uint32_t) + count * sizeof(struct fw_cfg_file);
+ filesbuf = g_malloc(dsize);
+ qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, filesbuf, dsize);
+ pdir_entry = (FWCfgFile *)(filesbuf + sizeof(uint32_t));
+ for (i = 0; i < count; ++i, ++pdir_entry) {
+ if (!strcmp(pdir_entry->name, filename)) {
+ *size = be32_to_cpu(pdir_entry->size);
+ *sel = be16_to_cpu(pdir_entry->select);
+ found = true;
+ break;
+ }
+ }
+
+ return found;
+}
+
/*
* The caller need check the return value. When the return value is
* nonzero, it means that some bytes have been transferred.
@@ -73,34 +104,20 @@ static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
* populated, it has received only a starting slice of the fw_cfg file.
*/
size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
- void *data, size_t buflen)
+ void *data, size_t buflen)
{
- uint32_t count;
- uint32_t i;
- unsigned char *filesbuf = NULL;
- size_t dsize;
- FWCfgFile *pdir_entry;
size_t filesize = 0;
+ uint32_t len;
+ uint16_t sel;
- qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, &count, sizeof(count));
- count = be32_to_cpu(count);
- dsize = sizeof(uint32_t) + count * sizeof(struct fw_cfg_file);
- filesbuf = g_malloc(dsize);
- qfw_cfg_get(fw_cfg, FW_CFG_FILE_DIR, filesbuf, dsize);
- pdir_entry = (FWCfgFile *)(filesbuf + sizeof(uint32_t));
- for (i = 0; i < count; ++i, ++pdir_entry) {
- if (!strcmp(pdir_entry->name, filename)) {
- uint32_t len = be32_to_cpu(pdir_entry->size);
- uint16_t sel = be16_to_cpu(pdir_entry->select);
- filesize = len;
- if (len > buflen) {
- len = buflen;
- }
- qfw_cfg_get(fw_cfg, sel, data, len);
- break;
+ if (find_pdir_entry(fw_cfg, filename, &sel, &len)) {
+ filesize = len;
+ if (len > buflen) {
+ len = buflen;
}
+ qfw_cfg_get(fw_cfg, sel, data, len);
}
- g_free(filesbuf);
+
return filesize;
}
--
2.45.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v6 2/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files
2025-01-20 4:38 [PATCH v6 0/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files Ani Sinha
2025-01-20 4:38 ` [PATCH v6 1/3] libqos/fw_cfg: refactor file directory iteraton to make it more reusable Ani Sinha
@ 2025-01-20 4:38 ` Ani Sinha
2025-01-27 7:12 ` Ani Sinha
2025-01-27 15:28 ` Fabiano Rosas
2025-01-20 4:38 ` [PATCH v6 3/3] tests/qtest/vmcoreinfo: add a unit test to exercize basic vmcoreinfo function Ani Sinha
2025-01-27 15:29 ` [PATCH v6 0/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files Fabiano Rosas
3 siblings, 2 replies; 9+ messages in thread
From: Ani Sinha @ 2025-01-20 4:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Gerd Hoffmann, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini
Cc: Ani Sinha, qemu-devel
At present, the libqos/fw_cfg.c library does not support the modern DMA
interface which is required to write to the fw_cfg files. It only uses the IO
interface. Implement read and write methods based on DMA. This will enable
developers to add tests that writes to the fw_cfg file(s). The structure of
the code is taken from edk2 fw_cfg implementation. It has been tested by
writing a qtest that writes to a fw_cfg file.
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
tests/qtest/libqos/fw_cfg.c | 139 ++++++++++++++++++++++++++++++++++++
tests/qtest/libqos/fw_cfg.h | 6 +-
2 files changed, 144 insertions(+), 1 deletion(-)
diff --git a/tests/qtest/libqos/fw_cfg.c b/tests/qtest/libqos/fw_cfg.c
index 22435873d9..0ab3959171 100644
--- a/tests/qtest/libqos/fw_cfg.c
+++ b/tests/qtest/libqos/fw_cfg.c
@@ -14,6 +14,8 @@
#include "qemu/osdep.h"
#include "fw_cfg.h"
+#include "malloc-pc.h"
+#include "libqos-malloc.h"
#include "../libqtest.h"
#include "qemu/bswap.h"
#include "hw/nvram/fw_cfg.h"
@@ -60,6 +62,60 @@ static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
qtest_writew(fw_cfg->qts, fw_cfg->base, key);
}
+static void qfw_cfg_dma_transfer(QFWCFG *fw_cfg, QOSState *qs, void *address,
+ uint32_t length, uint32_t control)
+{
+ FWCfgDmaAccess access;
+ uint32_t addr;
+ uint64_t guest_access_addr;
+ uint64_t gaddr;
+
+ /* create a data buffer in guest memory */
+ gaddr = guest_alloc(&qs->alloc, length);
+
+ if (control & FW_CFG_DMA_CTL_WRITE) {
+ qtest_bufwrite(fw_cfg->qts, gaddr, address, length);
+ }
+ access.address = cpu_to_be64(gaddr);
+ access.length = cpu_to_be32(length);
+ access.control = cpu_to_be32(control);
+
+ /* now create a separate buffer in guest memory for 'access' */
+ guest_access_addr = guest_alloc(&qs->alloc, sizeof(access));
+ qtest_bufwrite(fw_cfg->qts, guest_access_addr, &access, sizeof(access));
+
+ /* write lower 32 bits of address */
+ addr = cpu_to_be32((uint32_t)(uintptr_t)guest_access_addr);
+ qtest_outl(fw_cfg->qts, fw_cfg->base + 8, addr);
+
+ /* write upper 32 bits of address */
+ addr = cpu_to_be32((uint32_t)(uintptr_t)(guest_access_addr >> 32));
+ qtest_outl(fw_cfg->qts, fw_cfg->base + 4, addr);
+
+ g_assert(!(be32_to_cpu(access.control) & FW_CFG_DMA_CTL_ERROR));
+
+ if (control & FW_CFG_DMA_CTL_READ) {
+ qtest_bufread(fw_cfg->qts, gaddr, address, length);
+ }
+
+ guest_free(&qs->alloc, guest_access_addr);
+ guest_free(&qs->alloc, gaddr);
+}
+
+static void qfw_cfg_write_entry(QFWCFG *fw_cfg, QOSState *qs, uint16_t key,
+ void *buf, uint32_t len)
+{
+ qfw_cfg_select(fw_cfg, key);
+ qfw_cfg_dma_transfer(fw_cfg, qs, buf, len, FW_CFG_DMA_CTL_WRITE);
+}
+
+static void qfw_cfg_read_entry(QFWCFG *fw_cfg, QOSState *qs, uint16_t key,
+ void *buf, uint32_t len)
+{
+ qfw_cfg_select(fw_cfg, key);
+ qfw_cfg_dma_transfer(fw_cfg, qs, buf, len, FW_CFG_DMA_CTL_READ);
+}
+
static bool find_pdir_entry(QFWCFG *fw_cfg, const char *filename,
uint16_t *sel, uint32_t *size)
{
@@ -121,6 +177,89 @@ size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
return filesize;
}
+/*
+ * The caller need check the return value. When the return value is
+ * nonzero, it means that some bytes have been transferred.
+ *
+ * If the fw_cfg file in question is smaller than the allocated & passed-in
+ * buffer, then the first len bytes were read.
+ *
+ * If the fw_cfg file in question is larger than the passed-in
+ * buffer, then the return value explains how much was actually read.
+ *
+ * It is illegal to call this function if fw_cfg does not support DMA
+ * interface. The caller should ensure that DMA is supported before
+ * calling this function.
+ *
+ * Passed QOSState pointer qs must be initialized. qs->alloc must also be
+ * properly initialized.
+ */
+size_t qfw_cfg_read_file(QFWCFG *fw_cfg, QOSState *qs, const char *filename,
+ void *data, size_t buflen)
+{
+ uint32_t len = 0;
+ uint16_t sel;
+ uint32_t id;
+
+ g_assert(qs);
+ g_assert(filename);
+ g_assert(data);
+ g_assert(buflen);
+ /* check if DMA is supported since we use DMA for read */
+ id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
+ g_assert(id & FW_CFG_VERSION_DMA);
+
+ if (find_pdir_entry(fw_cfg, filename, &sel, &len)) {
+ if (len > buflen) {
+ len = buflen;
+ }
+ qfw_cfg_read_entry(fw_cfg, qs, sel, data, len);
+ }
+
+ return len;
+}
+
+/*
+ * The caller need check the return value. When the return value is
+ * nonzero, it means that some bytes have been transferred.
+ *
+ * If the fw_cfg file in question is smaller than the allocated & passed-in
+ * buffer, then the buffer has been partially written.
+ *
+ * If the fw_cfg file in question is larger than the passed-in
+ * buffer, then the return value explains how much was actually written.
+ *
+ * It is illegal to call this function if fw_cfg does not support DMA
+ * interface. The caller should ensure that DMA is supported before
+ * calling this function.
+ *
+ * Passed QOSState pointer qs must be initialized. qs->alloc must also be
+ * properly initialized.
+ */
+size_t qfw_cfg_write_file(QFWCFG *fw_cfg, QOSState *qs, const char *filename,
+ void *data, size_t buflen)
+{
+ uint32_t len = 0;
+ uint16_t sel;
+ uint32_t id;
+
+ g_assert(qs);
+ g_assert(filename);
+ g_assert(data);
+ g_assert(buflen);
+ /* write operation is only valid if DMA is supported */
+ id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
+ g_assert(id & FW_CFG_VERSION_DMA);
+
+ if (find_pdir_entry(fw_cfg, filename, &sel, &len)) {
+ if (len > buflen) {
+ len = buflen;
+ }
+ qfw_cfg_write_entry(fw_cfg, qs, sel, data, len);
+ }
+ return len;
+}
+
static void mm_fw_cfg_read(QFWCFG *fw_cfg, void *data, size_t len)
{
uint8_t *ptr = data;
diff --git a/tests/qtest/libqos/fw_cfg.h b/tests/qtest/libqos/fw_cfg.h
index b0456a15df..6d6ff09725 100644
--- a/tests/qtest/libqos/fw_cfg.h
+++ b/tests/qtest/libqos/fw_cfg.h
@@ -14,6 +14,7 @@
#define LIBQOS_FW_CFG_H
#include "../libqtest.h"
+#include "libqos.h"
typedef struct QFWCFG QFWCFG;
@@ -33,7 +34,10 @@ uint32_t qfw_cfg_get_u32(QFWCFG *fw_cfg, uint16_t key);
uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key);
size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
void *data, size_t buflen);
-
+size_t qfw_cfg_write_file(QFWCFG *fw_cfg, QOSState *qs, const char *filename,
+ void *data, size_t buflen);
+size_t qfw_cfg_read_file(QFWCFG *fw_cfg, QOSState *qs, const char *filename,
+ void *data, size_t buflen);
QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
--
2.45.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v6 2/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files
2025-01-20 4:38 ` [PATCH v6 2/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files Ani Sinha
@ 2025-01-27 7:12 ` Ani Sinha
2025-01-27 15:28 ` Fabiano Rosas
1 sibling, 0 replies; 9+ messages in thread
From: Ani Sinha @ 2025-01-27 7:12 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Gerd Hoffmann, Fabiano Rosas,
Laurent Vivier, Paolo Bonzini
Cc: qemu-devel
On Mon, Jan 20, 2025 at 10:09 AM Ani Sinha <anisinha@redhat.com> wrote:
>
> At present, the libqos/fw_cfg.c library does not support the modern DMA
> interface which is required to write to the fw_cfg files. It only uses the IO
> interface. Implement read and write methods based on DMA. This will enable
> developers to add tests that writes to the fw_cfg file(s). The structure of
> the code is taken from edk2 fw_cfg implementation. It has been tested by
> writing a qtest that writes to a fw_cfg file.
>
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
Final ping on this patch series ...
> ---
> tests/qtest/libqos/fw_cfg.c | 139 ++++++++++++++++++++++++++++++++++++
> tests/qtest/libqos/fw_cfg.h | 6 +-
> 2 files changed, 144 insertions(+), 1 deletion(-)
>
> diff --git a/tests/qtest/libqos/fw_cfg.c b/tests/qtest/libqos/fw_cfg.c
> index 22435873d9..0ab3959171 100644
> --- a/tests/qtest/libqos/fw_cfg.c
> +++ b/tests/qtest/libqos/fw_cfg.c
> @@ -14,6 +14,8 @@
>
> #include "qemu/osdep.h"
> #include "fw_cfg.h"
> +#include "malloc-pc.h"
> +#include "libqos-malloc.h"
> #include "../libqtest.h"
> #include "qemu/bswap.h"
> #include "hw/nvram/fw_cfg.h"
> @@ -60,6 +62,60 @@ static void mm_fw_cfg_select(QFWCFG *fw_cfg, uint16_t key)
> qtest_writew(fw_cfg->qts, fw_cfg->base, key);
> }
>
> +static void qfw_cfg_dma_transfer(QFWCFG *fw_cfg, QOSState *qs, void *address,
> + uint32_t length, uint32_t control)
> +{
> + FWCfgDmaAccess access;
> + uint32_t addr;
> + uint64_t guest_access_addr;
> + uint64_t gaddr;
> +
> + /* create a data buffer in guest memory */
> + gaddr = guest_alloc(&qs->alloc, length);
> +
> + if (control & FW_CFG_DMA_CTL_WRITE) {
> + qtest_bufwrite(fw_cfg->qts, gaddr, address, length);
> + }
> + access.address = cpu_to_be64(gaddr);
> + access.length = cpu_to_be32(length);
> + access.control = cpu_to_be32(control);
> +
> + /* now create a separate buffer in guest memory for 'access' */
> + guest_access_addr = guest_alloc(&qs->alloc, sizeof(access));
> + qtest_bufwrite(fw_cfg->qts, guest_access_addr, &access, sizeof(access));
> +
> + /* write lower 32 bits of address */
> + addr = cpu_to_be32((uint32_t)(uintptr_t)guest_access_addr);
> + qtest_outl(fw_cfg->qts, fw_cfg->base + 8, addr);
> +
> + /* write upper 32 bits of address */
> + addr = cpu_to_be32((uint32_t)(uintptr_t)(guest_access_addr >> 32));
> + qtest_outl(fw_cfg->qts, fw_cfg->base + 4, addr);
> +
> + g_assert(!(be32_to_cpu(access.control) & FW_CFG_DMA_CTL_ERROR));
> +
> + if (control & FW_CFG_DMA_CTL_READ) {
> + qtest_bufread(fw_cfg->qts, gaddr, address, length);
> + }
> +
> + guest_free(&qs->alloc, guest_access_addr);
> + guest_free(&qs->alloc, gaddr);
> +}
> +
> +static void qfw_cfg_write_entry(QFWCFG *fw_cfg, QOSState *qs, uint16_t key,
> + void *buf, uint32_t len)
> +{
> + qfw_cfg_select(fw_cfg, key);
> + qfw_cfg_dma_transfer(fw_cfg, qs, buf, len, FW_CFG_DMA_CTL_WRITE);
> +}
> +
> +static void qfw_cfg_read_entry(QFWCFG *fw_cfg, QOSState *qs, uint16_t key,
> + void *buf, uint32_t len)
> +{
> + qfw_cfg_select(fw_cfg, key);
> + qfw_cfg_dma_transfer(fw_cfg, qs, buf, len, FW_CFG_DMA_CTL_READ);
> +}
> +
> static bool find_pdir_entry(QFWCFG *fw_cfg, const char *filename,
> uint16_t *sel, uint32_t *size)
> {
> @@ -121,6 +177,89 @@ size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
> return filesize;
> }
>
> +/*
> + * The caller need check the return value. When the return value is
> + * nonzero, it means that some bytes have been transferred.
> + *
> + * If the fw_cfg file in question is smaller than the allocated & passed-in
> + * buffer, then the first len bytes were read.
> + *
> + * If the fw_cfg file in question is larger than the passed-in
> + * buffer, then the return value explains how much was actually read.
> + *
> + * It is illegal to call this function if fw_cfg does not support DMA
> + * interface. The caller should ensure that DMA is supported before
> + * calling this function.
> + *
> + * Passed QOSState pointer qs must be initialized. qs->alloc must also be
> + * properly initialized.
> + */
> +size_t qfw_cfg_read_file(QFWCFG *fw_cfg, QOSState *qs, const char *filename,
> + void *data, size_t buflen)
> +{
> + uint32_t len = 0;
> + uint16_t sel;
> + uint32_t id;
> +
> + g_assert(qs);
> + g_assert(filename);
> + g_assert(data);
> + g_assert(buflen);
> + /* check if DMA is supported since we use DMA for read */
> + id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
> + g_assert(id & FW_CFG_VERSION_DMA);
> +
> + if (find_pdir_entry(fw_cfg, filename, &sel, &len)) {
> + if (len > buflen) {
> + len = buflen;
> + }
> + qfw_cfg_read_entry(fw_cfg, qs, sel, data, len);
> + }
> +
> + return len;
> +}
> +
> +/*
> + * The caller need check the return value. When the return value is
> + * nonzero, it means that some bytes have been transferred.
> + *
> + * If the fw_cfg file in question is smaller than the allocated & passed-in
> + * buffer, then the buffer has been partially written.
> + *
> + * If the fw_cfg file in question is larger than the passed-in
> + * buffer, then the return value explains how much was actually written.
> + *
> + * It is illegal to call this function if fw_cfg does not support DMA
> + * interface. The caller should ensure that DMA is supported before
> + * calling this function.
> + *
> + * Passed QOSState pointer qs must be initialized. qs->alloc must also be
> + * properly initialized.
> + */
> +size_t qfw_cfg_write_file(QFWCFG *fw_cfg, QOSState *qs, const char *filename,
> + void *data, size_t buflen)
> +{
> + uint32_t len = 0;
> + uint16_t sel;
> + uint32_t id;
> +
> + g_assert(qs);
> + g_assert(filename);
> + g_assert(data);
> + g_assert(buflen);
> + /* write operation is only valid if DMA is supported */
> + id = qfw_cfg_get_u32(fw_cfg, FW_CFG_ID);
> + g_assert(id & FW_CFG_VERSION_DMA);
> +
> + if (find_pdir_entry(fw_cfg, filename, &sel, &len)) {
> + if (len > buflen) {
> + len = buflen;
> + }
> + qfw_cfg_write_entry(fw_cfg, qs, sel, data, len);
> + }
> + return len;
> +}
> +
> static void mm_fw_cfg_read(QFWCFG *fw_cfg, void *data, size_t len)
> {
> uint8_t *ptr = data;
> diff --git a/tests/qtest/libqos/fw_cfg.h b/tests/qtest/libqos/fw_cfg.h
> index b0456a15df..6d6ff09725 100644
> --- a/tests/qtest/libqos/fw_cfg.h
> +++ b/tests/qtest/libqos/fw_cfg.h
> @@ -14,6 +14,7 @@
> #define LIBQOS_FW_CFG_H
>
> #include "../libqtest.h"
> +#include "libqos.h"
>
> typedef struct QFWCFG QFWCFG;
>
> @@ -33,7 +34,10 @@ uint32_t qfw_cfg_get_u32(QFWCFG *fw_cfg, uint16_t key);
> uint64_t qfw_cfg_get_u64(QFWCFG *fw_cfg, uint16_t key);
> size_t qfw_cfg_get_file(QFWCFG *fw_cfg, const char *filename,
> void *data, size_t buflen);
> -
> +size_t qfw_cfg_write_file(QFWCFG *fw_cfg, QOSState *qs, const char *filename,
> + void *data, size_t buflen);
> +size_t qfw_cfg_read_file(QFWCFG *fw_cfg, QOSState *qs, const char *filename,
> + void *data, size_t buflen);
> QFWCFG *mm_fw_cfg_init(QTestState *qts, uint64_t base);
> void mm_fw_cfg_uninit(QFWCFG *fw_cfg);
> QFWCFG *io_fw_cfg_init(QTestState *qts, uint16_t base);
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v6 2/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files
2025-01-20 4:38 ` [PATCH v6 2/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files Ani Sinha
2025-01-27 7:12 ` Ani Sinha
@ 2025-01-27 15:28 ` Fabiano Rosas
1 sibling, 0 replies; 9+ messages in thread
From: Fabiano Rosas @ 2025-01-27 15:28 UTC (permalink / raw)
To: Ani Sinha, Philippe Mathieu-Daudé, Gerd Hoffmann,
Laurent Vivier, Paolo Bonzini
Cc: Ani Sinha, qemu-devel
Ani Sinha <anisinha@redhat.com> writes:
> At present, the libqos/fw_cfg.c library does not support the modern DMA
> interface which is required to write to the fw_cfg files. It only uses the IO
> interface. Implement read and write methods based on DMA. This will enable
> developers to add tests that writes to the fw_cfg file(s). The structure of
> the code is taken from edk2 fw_cfg implementation. It has been tested by
> writing a qtest that writes to a fw_cfg file.
>
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v6 3/3] tests/qtest/vmcoreinfo: add a unit test to exercize basic vmcoreinfo function
2025-01-20 4:38 [PATCH v6 0/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files Ani Sinha
2025-01-20 4:38 ` [PATCH v6 1/3] libqos/fw_cfg: refactor file directory iteraton to make it more reusable Ani Sinha
2025-01-20 4:38 ` [PATCH v6 2/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files Ani Sinha
@ 2025-01-20 4:38 ` Ani Sinha
2025-01-27 7:12 ` Ani Sinha
2025-01-27 15:28 ` Fabiano Rosas
2025-01-27 15:29 ` [PATCH v6 0/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files Fabiano Rosas
3 siblings, 2 replies; 9+ messages in thread
From: Ani Sinha @ 2025-01-20 4:38 UTC (permalink / raw)
To: Fabiano Rosas, Laurent Vivier, Paolo Bonzini,
Marc-André Lureau, Ani Sinha
Cc: qemu-devel
A new qtest is written that exercizes the fw-cfg DMA based read and write ops
to write values into vmcoreinfo fw-cfg file and read them back and verify that
they are the same.
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
MAINTAINERS | 2 +
tests/qtest/meson.build | 1 +
tests/qtest/vmcoreinfo-test.c | 90 +++++++++++++++++++++++++++++++++++
3 files changed, 93 insertions(+)
create mode 100644 tests/qtest/vmcoreinfo-test.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 846b81e3ec..57167c3c73 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3016,6 +3016,7 @@ F: include/system/device_tree.h
Dump
S: Supported
M: Marc-André Lureau <marcandre.lureau@redhat.com>
+R: Ani Sinha <anisinha@redhat.com>
F: dump/
F: hw/misc/vmcoreinfo.c
F: include/hw/misc/vmcoreinfo.h
@@ -3026,6 +3027,7 @@ F: qapi/dump.json
F: scripts/dump-guest-memory.py
F: stubs/dump.c
F: docs/specs/vmcoreinfo.rst
+F: tests/qtest/vmcoreinfo-test.c
Error reporting
M: Markus Armbruster <armbru@redhat.com>
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 94b28e5a53..fc669336a6 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -57,6 +57,7 @@ qtests_i386 = \
(config_all_devices.has_key('CONFIG_AHCI_ICH9') ? ['tco-test'] : []) + \
(config_all_devices.has_key('CONFIG_FDC_ISA') ? ['fdc-test'] : []) + \
(config_all_devices.has_key('CONFIG_I440FX') ? ['fw_cfg-test'] : []) + \
+ (config_all_devices.has_key('CONFIG_FW_CFG_DMA') ? ['vmcoreinfo-test'] : []) + \
(config_all_devices.has_key('CONFIG_I440FX') ? ['i440fx-test'] : []) + \
(config_all_devices.has_key('CONFIG_I440FX') ? ['ide-test'] : []) + \
(config_all_devices.has_key('CONFIG_I440FX') ? ['numa-test'] : []) + \
diff --git a/tests/qtest/vmcoreinfo-test.c b/tests/qtest/vmcoreinfo-test.c
new file mode 100644
index 0000000000..dcf3b5ae05
--- /dev/null
+++ b/tests/qtest/vmcoreinfo-test.c
@@ -0,0 +1,90 @@
+/*
+ * qtest vmcoreinfo test case
+ *
+ * Copyright Red Hat. 2025.
+ *
+ * Authors:
+ * Ani Sinha <anisinha@redhat.com>
+ *
+ * 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 "qemu/units.h"
+#include "libqos/libqos-pc.h"
+#include "libqtest.h"
+#include "standard-headers/linux/qemu_fw_cfg.h"
+#include "libqos/fw_cfg.h"
+#include "qemu/bswap.h"
+#include "hw/misc/vmcoreinfo.h"
+
+static void test_vmcoreinfo_write_basic(void)
+{
+ QFWCFG *fw_cfg;
+ QOSState *qs;
+ FWCfgVMCoreInfo info;
+ size_t filesize;
+ uint16_t guest_format;
+ uint16_t host_format;
+ uint32_t size;
+ uint64_t paddr;
+
+ qs = qtest_pc_boot("-device vmcoreinfo");
+ fw_cfg = pc_fw_cfg_init(qs->qts);
+
+ memset(&info, 0 , sizeof(info));
+ /* read vmcoreinfo and read back the host format */
+ filesize = qfw_cfg_read_file(fw_cfg, qs, FW_CFG_VMCOREINFO_FILENAME,
+ &info, sizeof(info));
+ g_assert_cmpint(filesize, ==, sizeof(info));
+
+ host_format = le16_to_cpu(info.host_format);
+ g_assert_cmpint(host_format, ==, FW_CFG_VMCOREINFO_FORMAT_ELF);
+
+ memset(&info, 0 , sizeof(info));
+ info.guest_format = cpu_to_le16(FW_CFG_VMCOREINFO_FORMAT_ELF);
+ info.size = cpu_to_le32(1 * MiB);
+ info.paddr = cpu_to_le64(0xffffff00);
+ info.host_format = cpu_to_le16(host_format);
+
+ /* write the values to the host */
+ filesize = qfw_cfg_write_file(fw_cfg, qs, FW_CFG_VMCOREINFO_FILENAME,
+ &info, sizeof(info));
+ g_assert_cmpint(filesize, ==, sizeof(info));
+
+ memset(&info, 0 , sizeof(info));
+
+ /* now read back the values we wrote and compare that they are the same */
+ filesize = qfw_cfg_read_file(fw_cfg, qs, FW_CFG_VMCOREINFO_FILENAME,
+ &info, sizeof(info));
+ g_assert_cmpint(filesize, ==, sizeof(info));
+
+ size = le32_to_cpu(info.size);
+ paddr = le64_to_cpu(info.paddr);
+ guest_format = le16_to_cpu(info.guest_format);
+
+ g_assert_cmpint(size, ==, 1 * MiB);
+ g_assert_cmpint(paddr, ==, 0xffffff00);
+ g_assert_cmpint(guest_format, ==, FW_CFG_VMCOREINFO_FORMAT_ELF);
+
+ pc_fw_cfg_uninit(fw_cfg);
+ qtest_shutdown(qs);
+}
+
+int main(int argc, char **argv)
+{
+ const char *arch = qtest_get_arch();
+
+ g_test_init(&argc, &argv, NULL);
+
+ if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
+ /* skip for non-x86 */
+ exit(EXIT_SUCCESS);
+ }
+
+ qtest_add_func("vmcoreinfo/basic-write",
+ test_vmcoreinfo_write_basic);
+
+ return g_test_run();
+}
--
2.45.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v6 3/3] tests/qtest/vmcoreinfo: add a unit test to exercize basic vmcoreinfo function
2025-01-20 4:38 ` [PATCH v6 3/3] tests/qtest/vmcoreinfo: add a unit test to exercize basic vmcoreinfo function Ani Sinha
@ 2025-01-27 7:12 ` Ani Sinha
2025-01-27 15:28 ` Fabiano Rosas
1 sibling, 0 replies; 9+ messages in thread
From: Ani Sinha @ 2025-01-27 7:12 UTC (permalink / raw)
To: Fabiano Rosas, Laurent Vivier, Paolo Bonzini,
Marc-André Lureau, Ani Sinha
Cc: qemu-devel
On Mon, Jan 20, 2025 at 10:09 AM Ani Sinha <anisinha@redhat.com> wrote:
>
> A new qtest is written that exercizes the fw-cfg DMA based read and write ops
> to write values into vmcoreinfo fw-cfg file and read them back and verify that
> they are the same.
>
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
Final ping on this patch.
> ---
> MAINTAINERS | 2 +
> tests/qtest/meson.build | 1 +
> tests/qtest/vmcoreinfo-test.c | 90 +++++++++++++++++++++++++++++++++++
> 3 files changed, 93 insertions(+)
> create mode 100644 tests/qtest/vmcoreinfo-test.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 846b81e3ec..57167c3c73 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3016,6 +3016,7 @@ F: include/system/device_tree.h
> Dump
> S: Supported
> M: Marc-André Lureau <marcandre.lureau@redhat.com>
> +R: Ani Sinha <anisinha@redhat.com>
> F: dump/
> F: hw/misc/vmcoreinfo.c
> F: include/hw/misc/vmcoreinfo.h
> @@ -3026,6 +3027,7 @@ F: qapi/dump.json
> F: scripts/dump-guest-memory.py
> F: stubs/dump.c
> F: docs/specs/vmcoreinfo.rst
> +F: tests/qtest/vmcoreinfo-test.c
>
> Error reporting
> M: Markus Armbruster <armbru@redhat.com>
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 94b28e5a53..fc669336a6 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -57,6 +57,7 @@ qtests_i386 = \
> (config_all_devices.has_key('CONFIG_AHCI_ICH9') ? ['tco-test'] : []) + \
> (config_all_devices.has_key('CONFIG_FDC_ISA') ? ['fdc-test'] : []) + \
> (config_all_devices.has_key('CONFIG_I440FX') ? ['fw_cfg-test'] : []) + \
> + (config_all_devices.has_key('CONFIG_FW_CFG_DMA') ? ['vmcoreinfo-test'] : []) + \
> (config_all_devices.has_key('CONFIG_I440FX') ? ['i440fx-test'] : []) + \
> (config_all_devices.has_key('CONFIG_I440FX') ? ['ide-test'] : []) + \
> (config_all_devices.has_key('CONFIG_I440FX') ? ['numa-test'] : []) + \
> diff --git a/tests/qtest/vmcoreinfo-test.c b/tests/qtest/vmcoreinfo-test.c
> new file mode 100644
> index 0000000000..dcf3b5ae05
> --- /dev/null
> +++ b/tests/qtest/vmcoreinfo-test.c
> @@ -0,0 +1,90 @@
> +/*
> + * qtest vmcoreinfo test case
> + *
> + * Copyright Red Hat. 2025.
> + *
> + * Authors:
> + * Ani Sinha <anisinha@redhat.com>
> + *
> + * 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 "qemu/units.h"
> +#include "libqos/libqos-pc.h"
> +#include "libqtest.h"
> +#include "standard-headers/linux/qemu_fw_cfg.h"
> +#include "libqos/fw_cfg.h"
> +#include "qemu/bswap.h"
> +#include "hw/misc/vmcoreinfo.h"
> +
> +static void test_vmcoreinfo_write_basic(void)
> +{
> + QFWCFG *fw_cfg;
> + QOSState *qs;
> + FWCfgVMCoreInfo info;
> + size_t filesize;
> + uint16_t guest_format;
> + uint16_t host_format;
> + uint32_t size;
> + uint64_t paddr;
> +
> + qs = qtest_pc_boot("-device vmcoreinfo");
> + fw_cfg = pc_fw_cfg_init(qs->qts);
> +
> + memset(&info, 0 , sizeof(info));
> + /* read vmcoreinfo and read back the host format */
> + filesize = qfw_cfg_read_file(fw_cfg, qs, FW_CFG_VMCOREINFO_FILENAME,
> + &info, sizeof(info));
> + g_assert_cmpint(filesize, ==, sizeof(info));
> +
> + host_format = le16_to_cpu(info.host_format);
> + g_assert_cmpint(host_format, ==, FW_CFG_VMCOREINFO_FORMAT_ELF);
> +
> + memset(&info, 0 , sizeof(info));
> + info.guest_format = cpu_to_le16(FW_CFG_VMCOREINFO_FORMAT_ELF);
> + info.size = cpu_to_le32(1 * MiB);
> + info.paddr = cpu_to_le64(0xffffff00);
> + info.host_format = cpu_to_le16(host_format);
> +
> + /* write the values to the host */
> + filesize = qfw_cfg_write_file(fw_cfg, qs, FW_CFG_VMCOREINFO_FILENAME,
> + &info, sizeof(info));
> + g_assert_cmpint(filesize, ==, sizeof(info));
> +
> + memset(&info, 0 , sizeof(info));
> +
> + /* now read back the values we wrote and compare that they are the same */
> + filesize = qfw_cfg_read_file(fw_cfg, qs, FW_CFG_VMCOREINFO_FILENAME,
> + &info, sizeof(info));
> + g_assert_cmpint(filesize, ==, sizeof(info));
> +
> + size = le32_to_cpu(info.size);
> + paddr = le64_to_cpu(info.paddr);
> + guest_format = le16_to_cpu(info.guest_format);
> +
> + g_assert_cmpint(size, ==, 1 * MiB);
> + g_assert_cmpint(paddr, ==, 0xffffff00);
> + g_assert_cmpint(guest_format, ==, FW_CFG_VMCOREINFO_FORMAT_ELF);
> +
> + pc_fw_cfg_uninit(fw_cfg);
> + qtest_shutdown(qs);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + const char *arch = qtest_get_arch();
> +
> + g_test_init(&argc, &argv, NULL);
> +
> + if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
> + /* skip for non-x86 */
> + exit(EXIT_SUCCESS);
> + }
> +
> + qtest_add_func("vmcoreinfo/basic-write",
> + test_vmcoreinfo_write_basic);
> +
> + return g_test_run();
> +}
> --
> 2.45.2
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v6 3/3] tests/qtest/vmcoreinfo: add a unit test to exercize basic vmcoreinfo function
2025-01-20 4:38 ` [PATCH v6 3/3] tests/qtest/vmcoreinfo: add a unit test to exercize basic vmcoreinfo function Ani Sinha
2025-01-27 7:12 ` Ani Sinha
@ 2025-01-27 15:28 ` Fabiano Rosas
1 sibling, 0 replies; 9+ messages in thread
From: Fabiano Rosas @ 2025-01-27 15:28 UTC (permalink / raw)
To: Ani Sinha, Laurent Vivier, Paolo Bonzini, Marc-André Lureau,
Ani Sinha
Cc: qemu-devel
Ani Sinha <anisinha@redhat.com> writes:
> A new qtest is written that exercizes the fw-cfg DMA based read and write ops
> to write values into vmcoreinfo fw-cfg file and read them back and verify that
> they are the same.
>
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v6 0/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files
2025-01-20 4:38 [PATCH v6 0/3] tests/qtest/libqos: add DMA support for writing and reading fw_cfg files Ani Sinha
` (2 preceding siblings ...)
2025-01-20 4:38 ` [PATCH v6 3/3] tests/qtest/vmcoreinfo: add a unit test to exercize basic vmcoreinfo function Ani Sinha
@ 2025-01-27 15:29 ` Fabiano Rosas
3 siblings, 0 replies; 9+ messages in thread
From: Fabiano Rosas @ 2025-01-27 15:29 UTC (permalink / raw)
To: Ani Sinha
Cc: Ani Sinha, kraxel, lvivier, pbonzini, armbru, philmd, berrange,
qemu-devel
Ani Sinha <anisinha@redhat.com> writes:
> This patchset enables DMA interface support for writing fw-cfg files in
> libqtest. The first patch is just a code refactoring so that fw-cfg
> directory parsing can be part of a separate helper function.
> The second patch is the actual patch that introduces two new apis for
> writing and reading fw-cfg files using DMA interface. The apis are
> tested by writing a new basic qtest for vmcoreinfo device in patch 3.
>
> $ MALLOC_PERTURB_=255 QTEST_QEMU_BINARY=./qemu-system-x86_64 /workspace/qemu-ani/build/tests/qtest/vmcoreinfo-test
> TAP version 13
> # random seed: R02S33ea0ae4568aae69a6191a97e6ea37fc
> 1..1
> # Start of x86_64 tests
> # Start of vmcoreinfo tests
> # starting QEMU: exec ./qemu-system-x86_64 -qtest unix:/tmp/qtest-267380.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-267380.qmp,id=char0 -mon chardev=char0,mode=control -display none -audio none -device vmcoreinfo -accel qtest
> ok 1 /x86_64/vmcoreinfo/basic-write
> # End of vmcoreinfo tests
> # End of x86_64 tests
>
> CI pipeline is green with this patch series: https://gitlab.com/anisinha/qemu/-/pipelines/1629670893
>
> checkpatch is clean:
>
> $ ./scripts/checkpatch.pl patches-v6/*
> Checking patches-v6/v6-0000-cover-letter.patch...
> total: 0 errors, 0 warnings, 0 lines checked
>
> patches-v6/v6-0000-cover-letter.patch has no obvious style problems and is ready for submission.
> Checking patches-v6/v6-0001-libqos-fw_cfg-refactor-file-directory-iteraton-to.patch...
> total: 0 errors, 0 warnings, 80 lines checked
>
> patches-v6/v6-0001-libqos-fw_cfg-refactor-file-directory-iteraton-to.patch has no obvious style problems and is ready for submission.
> Checking patches-v6/v6-0002-tests-qtest-libqos-add-DMA-support-for-writing-an.patch...
> total: 0 errors, 0 warnings, 175 lines checked
>
> patches-v6/v6-0002-tests-qtest-libqos-add-DMA-support-for-writing-an.patch has no obvious style problems and is ready for submission.
> Checking patches-v6/v6-0003-tests-qtest-vmcoreinfo-add-a-unit-test-to-exerciz.patch...
> total: 0 errors, 0 warnings, 111 lines checked
>
> patches-v6/v6-0003-tests-qtest-vmcoreinfo-add-a-unit-test-to-exerciz.patch has no obvious style problems and is ready for submission.
>
>
> cc: kraxel@redhat.com
> cc: farosas@suse.de
> cc: lvivier@redhat.com
> cc: pbonzini@redhat.com
> cc: armbru@redhat.com
> cc: philmd@linaro.org
> cc: berrange@redhat.com
> cc: qemu-devel@nongnu.org
> cc: farosas@suse.de
I'll give it the rest of the week and take this series via qtest-next
unless there's objections.
^ permalink raw reply [flat|nested] 9+ messages in thread