• L
    loader: fix handling of custom address spaces when adding ROM blobs · aa6c6ae8
    Laszlo Ersek 提交于
    * Commit 3e76099a ("loader: Allow a custom AddressSpace when loading
      ROMs") introduced the "Rom.as" field:
    
      (1) It modified the utility callers of rom_insert() to take "as" as a
          new parameter from *their* callers, and set "rom->as" from that
          parameter. The functions covered were rom_add_file() and
          rom_add_elf_program().
    
      (2) It also modified rom_insert() itself, to auto-assign
          "&address_space_memory", in case the external caller passed -- and
          the utility caller forwarded -- as=NULL.
    
      Except, commit 3e76099a forgot to update the third utility caller of
      rom_insert(), under point (1), namely rom_add_blob().
    
    * Later, commit 5e774eb3 ("loader: Add AddressSpace loading support
      to uImages") added the load_uimage_as() function, and the
      rom_add_blob_fixed_as() function-like macro, with the necessary changes
      elsewhere to propagate the new "as" parameter to rom_add_blob():
    
        load_uimage_as()
          load_uboot_image()
            rom_add_blob_fixed_as()
              rom_add_blob()
    
      At this point, the signature (and workings) of rom_add_blob() had been
      broken already, and the rom_add_blob_fixed_as() macro passed its "_as"
      parameter to rom_add_blob() as "callback_opaque". Given that the
      "fw_callback" parameter itself was set to NULL (correctly), this did no
      additional damage (the opaque arg would never be used), but ultimately
      it broke the new functionality of load_uimage_as().
    
    * The load_uimage_as() function would be put to use in one of the later
      patches, commit e481a1f6 ("generic-loader: Add a generic loader").
    
    * We can fix this only in a unified patch now. Append "AddressSpace *as"
      to the signature of rom_add_blob(), and handle the new parameter. Pass
      NULL from all current callers, except from rom_add_blob_fixed_as(),
      where "_as" has to be bumped to the proper position.
    
    * Note that rom_add_file() rejects the case when both "mr" and "as" are
      passed in as non-NULL. The action that this is apparently supposed to
      prevent is the
    
        rom->mr = mr;
    
      assignment (that's the only place where the "mr" parameter is used in
      rom_add_file()). In rom_add_blob() though, we have no "mr" parameter,
      and the actions done on the fw_cfg branch:
    
        if (fw_file_name && fw_cfg) {
            if (mc->rom_file_has_mr) {
                data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
                mr = rom->mr;
            } else {
                data = rom->data;
            }
    
      reflect those that are performed by rom_add_file() too (with mr==NULL):
    
        if (rom->fw_file && fw_cfg) {
            if ((!option_rom || mc->option_rom_has_mr) &&
                mc->rom_file_has_mr) {
                data = rom_set_mr(rom, OBJECT(fw_cfg), devpath);
            } else {
                data = rom->data;
            }
    
      Hence we need no additional restrictions in rom_add_blob().
    
    * Stable is not affected as both problematic commits appeared first in
      v2.8.0-rc0.
    
    Cc: "Michael S. Tsirkin" <mst@redhat.com>
    Cc: Alistair Francis <alistair.francis@xilinx.com>
    Cc: Igor Mammedov <imammedo@redhat.com>
    Cc: Michael Walle <michael@walle.cc>
    Cc: Paolo Bonzini <pbonzini@redhat.com>
    Cc: Peter Maydell <peter.maydell@linaro.org>
    Cc: Shannon Zhao <zhaoshenglong@huawei.com>
    Cc: qemu-arm@nongnu.org
    Cc: qemu-devel@nongnu.org
    Fixes: 3e76099a
    Fixes: 5e774eb3Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
    Reviewed-by: NAlistair Francis <alistair.francis@xilinx.com>
    Reviewed-by: NMichael S. Tsirkin <mst@redhat.com>
    Reviewed-by: NMichael S. Tsirkin <mst@redhat.com>
    Signed-off-by: NMichael S. Tsirkin <mst@redhat.com>
    aa6c6ae8
loader.c 32.7 KB