[U-Boot] [PATCH v4 1/8] x86: qemu: add fw_cfg support

[Simon Glass]

Hi Miao,

On 31 December 2015 at 01:42, Miao Yan <yanmiaobest at gmail.com> wrote:
> Hi Simon,
>
> 2015-12-31 13:07 GMT+08:00 Simon Glass <sjg at chromium.org>:
>> Hi Miao,
>>
>> On 30 December 2015 at 19:55, Miao Yan <yanmiaobest at gmail.com> wrote:
>>> The QEMU fw_cfg interface allows the guest to retrieve various
>>> data information from QEMU. For example, APCI/SMBios tables, number
>>> of online cpus, kernel data and command line, etc.
>>>
>>> This patch adds support for QEMU fw_cfg interface.
>>>
>>> Signed-off-by: Miao Yan <yanmiaobest at gmail.com>
>>> ---
>>> Changes in v4:
>>>   - cleanups
>>>   - change 'fw load' to take second parameter for initrd load address
>>>
>>>  arch/x86/cpu/qemu/Makefile |   2 +-
>>>  arch/x86/cpu/qemu/fw_cfg.c | 268 +++++++++++++++++++++++++++++++++++++++++++++
>>>  arch/x86/cpu/qemu/fw_cfg.h |  97 ++++++++++++++++
>>>  arch/x86/cpu/qemu/qemu.c   |   3 +
>>>  4 files changed, 369 insertions(+), 1 deletion(-)
>>>  create mode 100644 arch/x86/cpu/qemu/fw_cfg.c
>>>  create mode 100644 arch/x86/cpu/qemu/fw_cfg.h
>>
>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>
>> But a few nits...
[snip]
>>> +int qemu_fwcfg_online_cpus(void)
>>> +{
>>> +       uint16_t nb_cpus;
>>> +
>>> +       if (!fwcfg_present)
>>> +               return 1;
>>
>> -ENODEV
>
>
> Can we return 1 cpu if fw_cfg interface is not avaliable (which is
> quite unlikey),
> and print a warning maybe ? Because there has to be one cpu at least
> and returning
> -ENODEV wouldn't make much difference.

I see. In that case I think it is better if qemu_fwcfg_online_cpus()
returns an error and its caller can detect -ENODEV and use '1' as the
value. Then the decision as to how to deal with the error is handled
at the layer that is taking action.

>
> I'll fix rest of your comments and thanks for the review.
>
> Miao
>
[snip]