On 27/08/26 23:09, Peter Maydell wrote:
On Thu, 27 Aug 2026 at 17:12, Om <[email protected]> wrote:
On 27/08/26 15:38, Peter Maydell wrote:
On Sat, 22 Aug 2026 at 19:39, Om Barkare <[email protected]> wrote:
+static const VMStateDescription vmstate_pflash_sector_erase = {
+ .name = "pflash_cfi02_erase_sector",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = pflash_sector_erase_needed,
+ .fields = (const VMStateField[]) {
+ VMSTATE_INT32(sectors_to_erase, PFlashCFI02),
+ VMSTATE_UINT64(erase_time_remaining, PFlashCFI02),
+ VMSTATE_BITMAP(sector_erase_map, PFlashCFI02, 1, total_sectors),
+ VMSTATE_END_OF_LIST()
+ }
+
+};
Why did you choose to put this in a subsection ?
These states are accessed only if an erase is ongoing or if it is pending
which can happen if migration is performed mid-erase or when an erase
is suspended. As they are only accessed during an erase, so I thought I
would
put them in a subsection
Putting them into a subsection means extra code complexity here,
and a little bit of extra overhead in the on-the-wire format.
We usually only put things in a subsection if we need to do
that for migration compatibility (e.g. we added something to the
migration state later and so we want to avoid transmitting the
extra thing unless we really need to, so that we can still work
with an old QEMU that doesn't expect it). In this case the bitmap
is 1 bit per sector, and e.g. xilinx_zynq has a 128K sector size
and 64MB total size, for 512 sectors. That's only 64 bytes for the
bitmap, which is too small to be worth worrying about not sending.
So I think we should just include these fields directly in the
main vmstate.
-- PMM
Thanks for the review
Will make the changes and submit the patch
