Hi Paolo, On Wed, Sep 21, 2022 at 10:59 AM Paolo Bonzini <pbonz...@redhat.com> wrote: > Just a small comment, addr should be little-endian (see > fw_cfg_add_i32). It's not used outside x86_load_linux, so it is > possible to just use cpu_to_le32 there.
Oh, shucks: I thought about this and then forgot to do it. Thanks for catching it. > Also I think it's cleaner if a reset callback puts the value back to > zero. fw_cfg already has fw_cfg_machine_reset, so perhaps the easiest > way is to add a FWCfgCallback reset_cb argument to just > fw_cfg_add_bytes_callback. If I am missing something and it's not > necessary I can do the cpu_to_le32 change myself or wait for you; in > any case I'll wait for either your ack or a v5. Actually, the idea is for the change to be permanent, since that represents how the system has actually been booted. Are there substantial changes possible to the firmware configuration on fw_cfg_machine_reset() that setting this back how it was would make a difference? Or do we benefit from having some consistency? > By the way, does this supersede v1..v3 that use the new protocol (I'd > guess so from the presence of the same 2/2 patch), or are the two > patches doing belts-and-suspenders? This v4 supersedes everything else. Jason