Re: [Qemu-devel] pc_q35/i440fx: uses uninitialized variable 'ram_memory' if !xen_enabled()

[Stefano Stabellini]

On Mon, 29 Jul 2013, Peter Maydell wrote:
> Building QEMU with clang 3.3 results in the following warning:
> 
>     hw/i386/pc_q35.c:115:9: error: variable 'ram_memory' is used
>           uninitialized whenever 'if' condition is false
> [-Werror,-Wsometimes-uninitialized]
>         if (!xen_enabled()) {
>             ^~~~~~~~~~~~~~
>     hw/i386/pc_q35.c:134:32: note: uninitialized use occurs here
>         q35_host->mch.ram_memory = ram_memory;
>                                    ^~~~~~~~~~
>     hw/i386/pc_q35.c:115:5: note: remove the 'if' if its condition is always
>           true
>         if (!xen_enabled()) {
>         ^~~~~~~~~~~~~~~~~~~~
>     hw/i386/pc_q35.c:71:29: note: initialize the variable 'ram_memory' to
>           silence this warning
>         MemoryRegion *ram_memory;
>                                 ^
>                                  = NULL
>     1 error generated.
> 
> 
> This looks correct -- if xen_enabled() is true, we skip the call
> to pc_memory_init() which is what initializes ram_memory, but
> then later on we still stuff it into the q35_host->mch field,
> and as far as I can tell hw/pci-host/q35.c:mch_init() then uses
> the ram_memory value whether xen is enabled or not.
> 
> There also seems to be a similar use-of-uninitialized which
> clang doesn't spot in i440fx: hw/i386_pc_piix.c:pc_init1()
> doesn't init ram_memory if xen_enabled() is true, but we
> then pass ram_memory through i440fx_init()->i440fx_common_init()
> ->init_pam() which then tries to use it.
> 
> Can anybody who knows more about the x86 hw models and/or Xen
> suggest the correct fix for this?

The reason why ram_memory doesn't follow the usual initialization path
on Xen is that ram is allocated by the hypervisor (not QEMU), and QEMU
doesn't keep the whole memory mapped all the times (it can't).

If the issue is limited to the MemoryRegion pointer being NULL, that
would be easy to fix. Probably the best way would be to initialize the
pointer by passing it to xen_hvm_init and xen_ram_init, that already
allocates a "placeholder" MemoryRegion for the ram of the VM.

Then all the following memory accesses should work because they should
go through qemu_get_ram_ptr that calls xen_map_cache, that makes sure
that the guest memory is mapped in QEMU's address space.

In the case of pc_q35.c, we would also need to add:

if (xen_enabled() && xen_hvm_init() != 0) {
    fprintf(stderr, "xen hardware virtual machine initialisation failed\n");
    exit(1);
}

at the beginning of the function, like we already do in pc_init1.