> > QEMU_EFI reports the below error:
> > ProcessCmdAddPointer: invalid pointer value in "etc/acpi/tables"
> > OnRootBridgesConnected: InstallAcpiTables: Protocol Error
> >
> > Debugging shows that on first reboot(after hot-adding NVDIMM),
> > Qemu updates the etc/table-loader len,
> >
> > qemu_ram_resize()
> > fw_cfg_modify_file()
> > fw_cfg_modify_bytes_read()
> >
> > And in fw_cfg_modify_bytes_read() we set the "callback_opaque" for
> > the "key" entry to NULL. Because of this, on the second reboot,
> > virt_acpi_build_update() is called with a NULL "build_state" and
> > returns without updating the ACPI tables. This seems to be
> > upsetting the firmware.
> >
> > To fix this, don't change the callback_opaque in fw_cfg_modify_bytes_read().
>
> Fixes: bdbb5b1706d165 ("fw_cfg: add fw_cfg_machine_reset function")
> Acked-by: Igor Mammedov <imamm...@redhat.com>
>
> CCing Gerd to have a second set of eyes on it
Hmm. Original patch clears both 'callback_opaque' and 'callback' (where
'callback' used to be what 'select_cb' is today I think). Not fully
sure what the motivation was for that. Maybe because using both
fw_cfg_modify*() calls and a callback for update-on-read for a given
entry looks pointless. Should that be the case there are better ways to
catch that, like having fw_cfg_modify_bytes_read() throw an error in
case select_cb is not NULL instead of silently clearing the callback.
In any case clearing callback_opaque only is obviously wrong, so
Acked-by: Gerd Hoffmann <kra...@redhat.com>
take care,
Gerd
