On Fri, Feb 14, 2025 at 10:51:17AM +0100, Alexander Graf wrote:
> I also like to have dedicated register spaces per component. So even if you
> choose to make it a hard split, I think we're better off with 4k at
> 0xfef10000 for control and 64k at 0xfef20000 for the buffer for example.
Well, if we go for PIO transfer mode instead of device memory we don't
need map the buffer any more.
The control registers for the x86 variant are in io address space right
now (0x520, next to fw_cfg). We could place them in a mmio page @
0xfef10000 instead. Any preference, and if so, why?
First sketch below, on top of this series. No edk2 counterpart yet, so
untested beyond compiling the code.
take care,
Gerd
--------------------- cut here ------------------------
diff --git a/include/hw/uefi/var-service-api.h
b/include/hw/uefi/var-service-api.h
index 6765925d9ed0..99911e904652 100644
--- a/include/hw/uefi/var-service-api.h
+++ b/include/hw/uefi/var-service-api.h
@@ -21,16 +21,21 @@
#define UEFI_VARS_REG_MAGIC 0x00 /* 16 bit */
#define UEFI_VARS_REG_CMD_STS 0x02 /* 16 bit */
#define UEFI_VARS_REG_BUFFER_SIZE 0x04 /* 32 bit */
-#define UEFI_VARS_REG_BUFFER_ADDR_LO 0x08 /* 32 bit */
-#define UEFI_VARS_REG_BUFFER_ADDR_HI 0x0c /* 32 bit */
-#define UEFI_VARS_REGS_SIZE 0x10
+#define UEFI_VARS_REG_DMA_BUFFER_ADDR_LO 0x08 /* 32 bit */
+#define UEFI_VARS_REG_DMA_BUFFER_ADDR_HI 0x0c /* 32 bit */
+#define UEFI_VARS_REG_PIO_BUFFER_TRANSFER 0x10 /* 8-64 bit */
+#define UEFI_VARS_REG_PIO_BUFFER_CRC32C 0x18 /* 32 bit (read-only) */
+#define UEFI_VARS_REG_RESERVED 0x1c /* 32 bit */
+#define UEFI_VARS_REGS_SIZE 0x20
/* magic value */
#define UEFI_VARS_MAGIC_VALUE 0xef1
/* command values */
#define UEFI_VARS_CMD_RESET 0x01
-#define UEFI_VARS_CMD_MM 0x02
+#define UEFI_VARS_CMD_DMA_MM 0x02
+#define UEFI_VARS_CMD_PIO_MM 0x03
+#define UEFI_VARS_CMD_PIO_ZERO_OFFSET 0x04
/* status values */
#define UEFI_VARS_STS_SUCCESS 0x00
diff --git a/include/hw/uefi/var-service.h b/include/hw/uefi/var-service.h
index e078d2b0e68f..7dbede659a8f 100644
--- a/include/hw/uefi/var-service.h
+++ b/include/hw/uefi/var-service.h
@@ -56,6 +56,10 @@ struct uefi_vars_state {
QTAILQ_HEAD(, uefi_variable) variables;
QTAILQ_HEAD(, uefi_var_policy) var_policies;
+ /* pio transfer buffer */
+ uint32_t pio_xfer_offset;
+ uint8_t *pio_xfer_buffer;
+
/* boot phases */
bool end_of_dxe;
bool ready_to_boot;
diff --git a/hw/uefi/var-service-core.c b/hw/uefi/var-service-core.c
index 78a668e68fa2..a96b66934769 100644
--- a/hw/uefi/var-service-core.c
+++ b/hw/uefi/var-service-core.c
@@ -4,6 +4,7 @@
* uefi vars device
*/
#include "qemu/osdep.h"
+#include "qemu/crc32c.h"
#include "system/dma.h"
#include "migration/vmstate.h"
@@ -41,6 +42,7 @@ const VMStateDescription vmstate_uefi_vars = {
VMSTATE_UINT32(buf_size, uefi_vars_state),
VMSTATE_UINT32(buf_addr_lo, uefi_vars_state),
VMSTATE_UINT32(buf_addr_hi, uefi_vars_state),
+ /* TODO: pio xfer offset + buffer */
VMSTATE_BOOL(end_of_dxe, uefi_vars_state),
VMSTATE_BOOL(ready_to_boot, uefi_vars_state),
VMSTATE_BOOL(exit_boot_service, uefi_vars_state),
@@ -54,7 +56,7 @@ const VMStateDescription vmstate_uefi_vars = {
},
};
-static uint32_t uefi_vars_cmd_mm(uefi_vars_state *uv)
+static uint32_t uefi_vars_cmd_mm(uefi_vars_state *uv, bool dma_mode)
{
hwaddr dma;
mm_header *mhdr;
@@ -69,9 +71,13 @@ static uint32_t uefi_vars_cmd_mm(uefi_vars_state *uv)
}
/* read header */
- dma_memory_read(&address_space_memory, dma,
- uv->buffer, sizeof(*mhdr),
- MEMTXATTRS_UNSPECIFIED);
+ if (dma_mode) {
+ dma_memory_read(&address_space_memory, dma,
+ uv->buffer, sizeof(*mhdr),
+ MEMTXATTRS_UNSPECIFIED);
+ } else {
+ memcpy(uv->buffer, uv->pio_xfer_buffer, sizeof(*mhdr));
+ }
if (uadd64_overflow(sizeof(*mhdr), mhdr->length, &size)) {
return UEFI_VARS_STS_ERR_BAD_BUFFER_SIZE;
@@ -81,9 +87,15 @@ static uint32_t uefi_vars_cmd_mm(uefi_vars_state *uv)
}
/* read buffer (excl header) */
- dma_memory_read(&address_space_memory, dma + sizeof(*mhdr),
- uv->buffer + sizeof(*mhdr), mhdr->length,
- MEMTXATTRS_UNSPECIFIED);
+ if (dma_mode) {
+ dma_memory_read(&address_space_memory, dma + sizeof(*mhdr),
+ uv->buffer + sizeof(*mhdr), mhdr->length,
+ MEMTXATTRS_UNSPECIFIED);
+ } else {
+ memcpy(uv->buffer + sizeof(*mhdr),
+ uv->pio_xfer_buffer + sizeof(*mhdr),
+ mhdr->length);
+ }
memset(uv->buffer + size, 0, uv->buf_size - size);
/* dispatch */
@@ -113,9 +125,15 @@ static uint32_t uefi_vars_cmd_mm(uefi_vars_state *uv)
}
/* write buffer */
- dma_memory_write(&address_space_memory, dma,
- uv->buffer, sizeof(*mhdr) + mhdr->length,
- MEMTXATTRS_UNSPECIFIED);
+ if (dma_mode) {
+ dma_memory_write(&address_space_memory, dma,
+ uv->buffer, sizeof(*mhdr) + mhdr->length,
+ MEMTXATTRS_UNSPECIFIED);
+ } else {
+ memcpy(uv->pio_xfer_buffer + sizeof(*mhdr),
+ uv->buffer + sizeof(*mhdr),
+ sizeof(*mhdr) + mhdr->length);
+ }
return retval;
}
@@ -150,8 +168,13 @@ static uint32_t uefi_vars_cmd(uefi_vars_state *uv,
uint32_t cmd)
case UEFI_VARS_CMD_RESET:
uefi_vars_soft_reset(uv);
return UEFI_VARS_STS_SUCCESS;
- case UEFI_VARS_CMD_MM:
- return uefi_vars_cmd_mm(uv);
+ case UEFI_VARS_CMD_DMA_MM:
+ return uefi_vars_cmd_mm(uv, true);
+ case UEFI_VARS_CMD_PIO_MM:
+ return uefi_vars_cmd_mm(uv, false);
+ case UEFI_VARS_CMD_PIO_ZERO_OFFSET:
+ uv->pio_xfer_offset = 0;
+ return UEFI_VARS_STS_SUCCESS;
default:
return UEFI_VARS_STS_ERR_NOT_SUPPORTED;
}
@@ -161,6 +184,7 @@ static uint64_t uefi_vars_read(void *opaque, hwaddr addr,
unsigned size)
{
uefi_vars_state *uv = opaque;
uint64_t retval = -1;
+ void *xfer_ptr;
trace_uefi_reg_read(addr, size);
@@ -174,12 +198,37 @@ static uint64_t uefi_vars_read(void *opaque, hwaddr addr,
unsigned size)
case UEFI_VARS_REG_BUFFER_SIZE:
retval = uv->buf_size;
break;
- case UEFI_VARS_REG_BUFFER_ADDR_LO:
+ case UEFI_VARS_REG_DMA_BUFFER_ADDR_LO:
retval = uv->buf_addr_lo;
break;
- case UEFI_VARS_REG_BUFFER_ADDR_HI:
+ case UEFI_VARS_REG_DMA_BUFFER_ADDR_HI:
retval = uv->buf_addr_hi;
break;
+ case UEFI_VARS_REG_PIO_BUFFER_TRANSFER:
+ if (uv->pio_xfer_offset + size > uv->buf_size) {
+ retval = 0;
+ break;
+ }
+ xfer_ptr = uv->pio_xfer_buffer + uv->pio_xfer_offset;
+ switch (size) {
+ case 1:
+ retval = *(uint8_t *)xfer_ptr;
+ break;
+ case 2:
+ retval = *(uint16_t *)xfer_ptr;
+ break;
+ case 4:
+ retval = *(uint32_t *)xfer_ptr;
+ break;
+ case 8:
+ retval = *(uint64_t *)xfer_ptr;
+ break;
+ }
+ uv->pio_xfer_offset += size;
+ break;
+ case UEFI_VARS_REG_PIO_BUFFER_CRC32C:
+ retval = crc32c(0xffffffff, uv->pio_xfer_buffer, uv->pio_xfer_offset);
+ break;
}
return retval;
}
@@ -187,6 +236,7 @@ static uint64_t uefi_vars_read(void *opaque, hwaddr addr,
unsigned size)
static void uefi_vars_write(void *opaque, hwaddr addr, uint64_t val, unsigned
size)
{
uefi_vars_state *uv = opaque;
+ void *xfer_ptr;
trace_uefi_reg_write(addr, val, size);
@@ -200,14 +250,40 @@ static void uefi_vars_write(void *opaque, hwaddr addr,
uint64_t val, unsigned si
}
uv->buf_size = val;
g_free(uv->buffer);
+ g_free(uv->pio_xfer_buffer);
uv->buffer = g_malloc(uv->buf_size);
+ uv->pio_xfer_buffer = g_malloc(uv->buf_size);
break;
- case UEFI_VARS_REG_BUFFER_ADDR_LO:
+ case UEFI_VARS_REG_DMA_BUFFER_ADDR_LO:
uv->buf_addr_lo = val;
break;
- case UEFI_VARS_REG_BUFFER_ADDR_HI:
+ case UEFI_VARS_REG_DMA_BUFFER_ADDR_HI:
uv->buf_addr_hi = val;
break;
+ case UEFI_VARS_REG_PIO_BUFFER_TRANSFER:
+ if (uv->pio_xfer_offset + size > uv->buf_size) {
+ break;
+ }
+ xfer_ptr = uv->pio_xfer_buffer + uv->pio_xfer_offset;
+ switch (size) {
+ case 1:
+ *(uint8_t *)xfer_ptr = val;
+ break;
+ case 2:
+ *(uint16_t *)xfer_ptr = val;
+ break;
+ case 4:
+ *(uint32_t *)xfer_ptr = val;
+ break;
+ case 8:
+ *(uint64_t *)xfer_ptr = val;
+ break;
+ }
+ uv->pio_xfer_offset += size;
+ break;
+ case UEFI_VARS_REG_PIO_BUFFER_CRC32C:
+ default:
+ break;
}
}
