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

[Petr Berky]

On 03/14/2017 09:46 PM, Laszlo Ersek wrote:
> On 03/14/17 21:33, petr.berky at email.cz wrote:
>> From 405de6e571a2bf332452a17ae98f7b3a0613365e Mon Sep 17 00:00:00 2001
>> From: Petr Berky <petr.berky at email.cz>
>> 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 <petr.berky at email.cz>
>> ---
>>  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
>

I think you are right. I will prepare a second patch