Re: [PATCH 2/3] optionrom: add a DMA-enabled multiboot ROM

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 10/20/21 16:02, Paolo Bonzini wrote:
> From: Marcus Hähnel <marcus.haehnel@kernkonzept.com>
> 
> Add a new option rom for the multiboot loader, using DMA transfers to copy
> data instead of "rep insb".
> 
> This significantly lowers QEMU's startup latency by a factor of about 40,
> for example, going from 30sec to 0.8sec when loading modules of 120MB
> in size.
> 
> Signed-off-by: Marcus Hähnel <marcus.haehnel@kernkonzept.com>
> Signed-off-by: Adam Lackorzynski <adam@l4re.org>
> [Modified to keep the non-DMA code depending on #ifdef USE_FW_CFG_DMA;
>  do not write below stack. - Paolo]
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  pc-bios/meson.build               |   1 +
>  pc-bios/multiboot_dma.bin         | Bin 0 -> 1024 bytes
>  pc-bios/optionrom/Makefile        |   4 +-
>  pc-bios/optionrom/multiboot.S     |   4 +-
>  pc-bios/optionrom/multiboot_dma.S |   2 +
>  pc-bios/optionrom/optionrom.h     |  66 ++++++++++++++++++++++++++++++
>  6 files changed, 72 insertions(+), 5 deletions(-)
>  create mode 100644 pc-bios/multiboot_dma.bin
>  create mode 100644 pc-bios/optionrom/multiboot_dma.S

> diff --git a/pc-bios/multiboot_dma.bin b/pc-bios/multiboot_dma.bin
> new file mode 100644
> index 0000000000000000000000000000000000000000..c0e2c3102a3358207c61d3ae113524fb6007abc3
> GIT binary patch
> literal 1024
> zcmd^-v1=1i9LIlmUTlcNU1}{N5`u&{DAHSmq6iKdix;GHRxaNnR0A<Lc978Fk#IvN
> zaS+5wQE+fL&*iQWa#czc94ZyL>R{3q;@~8~^LMo;4s!p158nI!-tWur_ul*P<{!&%
> z=%3>xm5f`4Bq!!)`Rkwf;(oFd*qcAb=mhXX1)X>Bw-tylkUpR_ETXkH1AnO4L$nJ|
> zWJui_M1kmKmTID)P`mI^=HM`~{VB1t1l_Ye=Lor4X5{8Gd)zzw_o4<6zLQJ!dr<pW
> z;7%~ysSHL?*HHx*O%k}?fCyRL<7_RWt(4;Aqn}X}S>MolrKNZG2Z|kTOyaJIhYFc^
> zFhhuQ9`r5fQLCrm#T4^_QylQ>8kgs;ZX9bIEiXb`8AIxu;?h~d%9izBkKht%WM1G*
> z^E{W9(OT9dt5in2qF}|dPH>dL>{=sV#-U1<quUb@YkIW%niI?8-7fCz#695e<V=WZ
> zrVCY?W~`3Hw@^=cHALr#9F2GOTYJ;??9d*-Vbsi+;lz|<$jMX#;lr6ov3qKNLH7(W
> z+>yFoWnOtw14D$&k*SUtsT%wS`ki@#&rUrnmtuDv`PtJm(Kg?H$Pdc0CL@YCy4R<D
> pT|LnIbg(BnP0#tqRx5M!bkkaD-nd?`H;YU4Yi6yHwD{k({tHC8FAx9#

Ideally this should be generated on CI as artifact, archived
and we commit the CI generated file.

> diff --git a/pc-bios/optionrom/optionrom.h b/pc-bios/optionrom/optionrom.h
> index a2b612f1a7..8d74c0ddf3 100644
> --- a/pc-bios/optionrom/optionrom.h
> +++ b/pc-bios/optionrom/optionrom.h
> @@ -37,6 +37,17 @@
>  #define BIOS_CFG_IOPORT_CFG	0x510
>  #define BIOS_CFG_IOPORT_DATA	0x511
>  
> +#define FW_CFG_DMA_CTL_ERROR   0x01
> +#define FW_CFG_DMA_CTL_READ    0x02
> +#define FW_CFG_DMA_CTL_SKIP    0x04
> +#define FW_CFG_DMA_CTL_SELECT  0x08
> +#define FW_CFG_DMA_CTL_WRITE   0x10
> +
> +#define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */
> +
> +#define BIOS_CFG_DMA_ADDR_HIGH  0x514
> +#define BIOS_CFG_DMA_ADDR_LOW   0x518
> +
>  /* Break the translation block flow so -d cpu shows us values */
>  #define DEBUG_HERE \
>  	jmp		1f;				\
> @@ -62,6 +73,61 @@
>  	bswap		%eax
>  .endm
>  
> +
> +/*
> + * Read data from the fw_cfg device using DMA.
> + * Clobbers:	%edx, %eax, ADDR, SIZE, memory[%esp-16] to memory[%esp]
> + */
> +.macro read_fw_dma VAR, SIZE, ADDR
> +        /* Address */
> +	bswapl		\ADDR
> +	pushl		\ADDR
> +
> +	/* We only support 32 bit target addresses */
> +	xorl		%eax, %eax
> +	pushl		%eax
> +	mov		$BIOS_CFG_DMA_ADDR_HIGH, %dx
> +	outl		%eax, (%dx)
> +
> +	/* Size */
> +	bswapl		\SIZE
> +	pushl		\SIZE
> +
> +        /* Control */

Indent off.

> +	movl		$(\VAR << 16) | (FW_CFG_DMA_CTL_READ | FW_CFG_DMA_CTL_SELECT), %eax
> +	bswapl		%eax
> +	pushl		%eax
> +
> +	movl		%esp, %eax /* Address of the struct we generated */
> +	bswapl		%eax
> +	mov		$BIOS_CFG_DMA_ADDR_LOW, %dx
> +	outl		%eax, (%dx) /* Initiate DMA */
> +
> +1:  mov		(%esp), %eax /* Wait for completion */
> +	bswapl		%eax
> +	testl		$~FW_CFG_DMA_CTL_ERROR, %eax
> +	jnz		1b
> +       addl            $16, %esp

Indent off.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>