On Mon, 2 Mar 2026, Peter Xu wrote:
On Mon, Mar 02, 2026 at 04:20:10PM +0100, BALATON Zoltan wrote:
On Mon, 2 Mar 2026, Akihiko Odaki wrote:
On 2026/03/02 21:26, BALATON Zoltan wrote:
On Mon, 2 Mar 2026, Akihiko Odaki wrote:
On 2026/02/26 1:30, Peter Xu wrote:
On Tue, Feb 24, 2026 at 06:44:58PM +0100, BALATON Zoltan wrote:
On Tue, 24 Feb 2026, Peter Xu wrote:
On Thu, Feb 05, 2026 at 02:13:18AM +0100, BALATON Zoltan wrote:
v4:
- separate patch converting Sun machines from
memory_region_init_ram_nomigrate
- split helper to init ram into two functions:
setup and error_propagate
- also use memory_region_init_io in memory_region_init_ram_device_ptr
v3:
- rebased on master after some patches were merged
- drop some more line from memory-region-housekeeping.cocci
- added comment to explain what factored out helper does
- some more clean ups included
BALATON Zoltan (8):
hw/display/{cg3,tcx}: Do not use memory_region_init_rom_nomigrate()
memory: Remove memory_region_init_rom_nomigrate()
sun4m,sun4u,tcx: Do not use memory_region_init_ram_nomigrate()
memory: Remove memory_region_init_ram_nomigrate()
Could you help explain why we need to remove the
above two small helpers?
Ideally we should remove all of the nomigrate variants as there are only
about 3 uses left, everything else is already moved to
the init_{ram,rom}
variants that register the memory region for migration.
The is probably one
legitimate use that is served by memory_region_init_flags_nomigrate and
other 3 uses are the xtfpga which I think is just old code and does not
support migration so it will be converted in the second series to
memory_region_new as the maintainers did not say anyting to that so far.
Then there's one in hw/display/vga but that's gated
behind a compatibility
switch that's only set in 2.12 (global-vmstate=true) and
earlier. This is
not active any more but cannot yet be removed as there
are eariler compat
props still need cleaning up but the last visible
machine version is 5.1 now
so this is long deprecated and not used. And there are
these Sun machines
that I've converted in the first version but Mark said
he'd like to keep the
current behaviour until it's the last user left. See:
https://
patchew.org/QEMU/[email protected]/
b530cf54609b853c6b34306fd4aa3ce2f8087d05.1769703287.git.bala...@eik.bme.hu/
Not saying that we can't remove them, but I don't yet see the point of
open-code those readonly=true setup either, or any
problem with having the
smaller helpers when ram_flags=0. Small helpers
sometimes help readability.
Some of them are going backwards againist what the cocci scripts were
suggesting previously, like:
- memory_region_init_ram_nomigrate(E1, E2, E3, E4, E5);
+ memory_region_init_rom_nomigrate(E1, E2, E3, E4, E5);
... WHEN != E1
- memory_region_set_readonly(E1, true);
So you suggest open-code instead. Why?
This is temporary until the usage in vga is gone with
hw_compat_2_12 then
the Sun machines will be the last users so they can also be converted to
init_{ram,rom} (unless Mark changes his mind and agrees
changing these now
as was in the first version).
Is it required for your follow up cleanups?
It makes those simpler as then we don't need to add memory_region_new
variants for the nomigrate versions that we would remove later anyway.
Personally I don't see the memory_region_new() whole thing solid yet, but
still during review. I shared my thoughts. So far, I'm not fully
convinced, but I also don't have the full picture to say no, either. We
can wait for others to chime in with more thoughts.
For this series, I would prefer if we can discuss it separately from the
other work.
Removing _nomigrate() variances makes sense to me in general, it means we
want to by default suggest new devices only use RAM regions that is
migratable, with proper owner device specified along with
the ramblock name
(as part of migration API).
However, we have two sets of these _nomigrate APIs:
(1) memory_region_init_r[ao]m_nomigrate
(2) memory_region_init_r[ao]m_flags_nomigrate
What I am still confused about is why this series used (2) to replace (1)
rather than removing both. I don't understand how this helps if we still
have (2) around: it means new devices at least can still use
(2) to bypass
migration.
There's actually one way to enforce migratable RAMs, taking
ROM as example,
it could be:
memory_region_init_rom_internal(..., global_vmstate=XXX)
Then memory_region_init_rom() should use that, setting
global_vmstate=off.
Then we can unexport/remove memory_region_init_rom_[flag_]nomigrate() but
use that _internal() in relevant paths; we should also mark
the _internal()
functions to say "new code is suggested to use the normal
version". But I
do not know if that is a slight overkill just for this..
If the other series (to avoid introducing new APIs) is the
only reason for
this part, then IMHO this part of the series needs to be in the other
series, not this one. The last cleanup patches look more self contained
OTOH to me, if the whole purpose of the latter half was code
dedup, then it
makes more sense at least to me.
+1 for what Peter Xu suggested. Most part of this series looks
independently useful, but I don't think the removal of
memory_region_init_ram_nomigrate() is.
But if there are no useful uses why keep it? This version shows that
converting the Sun machines as Peter Maydell suggested only leaves
very few uses that can be removed eventually too after some further
clean up that I don't want to take up now. Do you have any usage in
mind that needs these nomigrate variants?
Well, my threshold for "useful" is > 1 uses, but maintainers may have
other preferences.
Personally I share the same preference..
When >1 it means we can dedup some code with a function and I normally
think it worthwhile. That's also why I don't understand why we need to
open code _set_readonly() in some of this series.
Have you read the latest v5 at all? No open coding of set_readonly or
useful uses of memory_region_init_r[ao]m_nomigrate left if we accept
converting the Sun machines now to memory_region_init_r[ao]m which they
should really use as everything else does that and these are the last
remaining uses of these nomigrate variants apart from the few special
cases. It seems these Sun machines were left behind when everything else
converted away from global vmstate and use the normal
memory_region_init_r[ao]m where the _nomigrate were left for backward
compatibility during API conversion. So it's time to finish that
conversion at last.
Even if for the sake of a new set of APIs, it doesn't need to provide every
single API that we have had in the old set if we don't need it, IMHO.
After this series when converting the Sun machines as in v5 the only
nomigrate variant left is memory_region_init_ram_flags_nomigrate that is
equivalent to memory_region_init_ram_nomigrate with flags=0 which appears
outside of memory.c only in hw/xtensa/xtfpga.c (that will be converted in
the next series to memory_region_new) and in hw/display/vga.c that can be
removed when the hw_compat_2_12 is removed. There are two more uses of
memory_region_init_ram_flags_nomigrate in backends/hostmem-ram.c and
hw/m68k/next-cube.c but those have non-0 flags parameter. Therefore I think
there's no need to keep other nomigrate variants than
memory_region_init_ram_flags_nomigrate as the only two uses outside of
memory.c that would need memory_region_init_ram_nomigrate are to be removed
soon and can use flags=0 until then.
I tried to explain here that the two uses left that could use
memory_region_init_ram_nomigrate will be gone shortly so IMO not worth
keeping this function around for that.
I don't know if the other two uses with non-0 flags can be replaced and
memory_region_init_ram_flags_nomigrate made internal to memory.c but that's
beyond the scope of this series and could be considered separately. Creating
nomigrate memory region is not something that should be done or need several
functions and the very few cases where this might be needed can use
memory_region_init_ram_flags_nomigrate.
The hostmem-ram.c one is slightly special, when created as an object it
still doesn't know its parent device.
Yes so there might be some special cases but for those it's enough to have
a single nomigrate variant which this series proposes.
I still think it might be helpful if you can split your "cleanup" patches
out of this series. Note that softfreeze will be next week.
This series _is_ the clean up part split from the add memory_region_new
functions series so these are all clean ups and the rest is in the other
series. I don't theres much more to split. Maybe you missed some patch
emails. Here's the clean up series:
https://patchew.org/QEMU/[email protected]/
and here's the rest of my original series based on these clean ups that
continue with adding and using the memory_region_new functions added in
that series:
https://patchew.org/QEMU/[email protected]/
I'm not sure what do you want to split.
Regards,
BALATON Zoltan
