Re: [SeaBIOS] [PATCH] config: Add function to check if fw_cfg exists

[Laszlo Ersek]

On 03/14/17 21:33, [e-mail] wrote:
> From 405de6e571a2bf332452a17ae98f7b3a0613365e Mon Sep 17 00:00:00 2001
> From: Petr Berky 
> Date: Tue, 14 Mar 2017 20:30:52 +0100
> Subject: [PATCH] config: Add function to check if fw_cfg exists
>
> It was found qemu_get_present_cpus_count may return impossible
> number of cpus because of not checking if fw_cfg exists before
> using it. That  may lead to undefined behavior of emulator,
> in particular Bochs that freezes.
> Signed-off-by: Petr Berky
>
> src/fw/paravirt.c | 28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> index 707502d..b2cfc23 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -220,6 +220,21 @@ qemu_cfg_select(u16 f)
>      outw(f, PORT_QEMU_CFG_CTL);
>  }
>
> +static int
> +qemu_cfg_check_signature(void)
> +{
> int i;
> char *sig = "QEMU";
> qemu_cfg_select(QEMU_CFG_SIGNATURE);
> for (i = 0; i < 4; i++) {
>    if (inb(PORT_QEMU_CFG_DATA) != sig[i]) {
>        return -1;
>    }
> }
> return 0;
>
> +}
>
> static void
>  qemu_cfg_dma_transfer(void *address, u32 length, u32 control)
>  {
> @@ -392,7 +407,9 @@ u16
>  qemu_get_present_cpus_count(void)
>  {
>      u16 smp_count = 0;
> qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count));
> if (qemu_cfg_check_signature() == 0) {
>    qemu_cfg_read_entry(&smp_count, QEMU_CFG_NB_CPUS, sizeof(smp_count));
> }
> u16 cmos_cpu_count = rtc_read(CMOS_BIOS_SMP_COUNT) + 1;
> if (smp_count < cmos_cpu_count) {
>     smp_count = cmos_cpu_count;
>
> @@ -563,12 +580,9 @@ void qemu_cfg_init(void)
>          return;
>  // Detect fw_cfg interface.
> qemu_cfg_select(QEMU_CFG_SIGNATURE);
> char *sig = "QEMU";
> int i;
> for (i = 0; i < 4; i++)
>    if (inb(PORT_QEMU_CFG_DATA) != sig[i])
>        return;
>
> if (qemu_cfg_check_signature() != 0) {
>    return;
>
> }
>
> dprintf(1, "Found QEMU fw_cfg\n");

"src/fw/paravirt.c" already has an extern function called
qemu_cfg_dma_enabled(), whic is based on the static global variable
"cfg_dma_enabled", which is set in qemu_cfg_init().

The above is about the DMA interface for fw_cfg. I think it would be a
"natural" extension to add a similar global variable and helper function
(called qemu_cfg_enabled()) for the basic fw_cfg presence as well.

Then qemu_cfg_check_signature() would not be necessary in this patch.

Just an idea of course.

Laszlo