On Thu, 23 May 2024 at 02:48, Song Gao <gaos...@loongson.cn> wrote:
>
> From: Bibo Mao <maob...@loongson.cn>
>
> Memory map table for fwcfg is used for UEFI BIOS, UEFI BIOS uses the first
> entry from fwcfg memory map as the first memory HOB, the second memory HOB
> will be used if the first memory HOB is used up.
>
> Memory map table for fwcfg does not care about numa node, however in
> generic the first memory HOB is part of numa node0, so that runtime
> memory of UEFI which is allocated from the first memory HOB is located
> at numa node0.
>
> Signed-off-by: Bibo Mao <maob...@loongson.cn>
> Reviewed-by: Song Gao <gaos...@loongson.cn>
> Message-Id: <20240515093927.3453674-4-maob...@loongson.cn>
> Signed-off-by: Song Gao <gaos...@loongson.cn>
Hi; Coverity points out a possible issue with this code
(CID 1546441):
> +static void fw_cfg_add_memory(MachineState *ms)
> +{
> + hwaddr base, size, ram_size, gap;
> + int nb_numa_nodes, nodes;
> + NodeInfo *numa_info;
> +
> + ram_size = ms->ram_size;
> + base = VIRT_LOWMEM_BASE;
> + gap = VIRT_LOWMEM_SIZE;
> + nodes = nb_numa_nodes = ms->numa_state->num_nodes;
> + numa_info = ms->numa_state->nodes;
> + if (!nodes) {
> + nodes = 1;
> + }
> +
> + /* add fw_cfg memory map of node0 */
> + if (nb_numa_nodes) {
> + size = numa_info[0].node_mem;
> + } else {
> + size = ram_size;
> + }
> +
> + if (size >= gap) {
> + memmap_add_entry(base, gap, 1);
> + size -= gap;
> + base = VIRT_HIGHMEM_BASE;
> + gap = ram_size - VIRT_LOWMEM_SIZE;
In this if() statement we set 'gap'...
> + }
> +
> + if (size) {
> + memmap_add_entry(base, size, 1);
> + base += size;
> + }
> +
> + if (nodes < 2) {
> + return;
> + }
> +
> + /* add fw_cfg memory map of other nodes */
> + size = ram_size - numa_info[0].node_mem;
> + gap = VIRT_LOWMEM_BASE + VIRT_LOWMEM_SIZE;
...but then later here we unconditionally overwrite 'gap',
without ever using it in between, making the previous
assignment useless.
What was the intention here ?
thanks
-- PMM
