On Thu, 30 Oct 2025, Mark Cave-Ayland wrote:
On 29/10/2025 13:25, BALATON Zoltan wrote:
On Wed, 29 Oct 2025, BALATON Zoltan wrote:
On Wed, 29 Oct 2025, Akihiko Odaki wrote:
On 2025/10/29 6:28, BALATON Zoltan wrote:
On Wed, 29 Oct 2025, Akihiko Odaki wrote:
On 2025/10/28 21:59, BALATON Zoltan wrote:
On Tue, 28 Oct 2025, Philippe Mathieu-Daudé wrote:
On 27/10/25 20:47, BALATON Zoltan wrote:
On Mon, 27 Oct 2025, Philippe Mathieu-Daudé wrote:
On 25/10/25 01:31, BALATON Zoltan wrote:
These memory windows are a result of the address decoding in the
Articia S north bridge so better model it there and not in board
code.
Suggested-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: BALATON Zoltan <[email protected]>
---
hw/pci-host/articia.c | 15 ++++++++++++++-
hw/ppc/amigaone.c | 28 +++++-----------------------
hw/ppc/pegasos2.c | 13 -------------
3 files changed, 19 insertions(+), 37 deletions(-)
[...]
It looks like we won't be able to come to an agreement before the freeze
and I don't have time now to change this patch but don't want to miss the
release with this series that finishes pegasos renaming because of this.
So for this patch I'd say since this is already how it is now and it does
not make it worse and this object is not user creatable anyway so cannot
leak please take it as it is and we'll do a clean up later after we finish
discussion.
As for all of these files I'm the maintainer let me make an executive
decision here to keep this patch without Philippe's reviewed-by for now to
be able to move on with this series before the freeze. Fixing the
theoretical leak can be done on top and since that's a fix it can be done
during soft freeze that would give us more time. So Harsh please go ahead
and merge this series too if there are no other concerns. I'll then address
this later together with other similar issues elsewhere.
I think the issue here is that several people have now raised concerns as to
the way you are trying to use MemoryRegions (both here and in the raven
series): simply put, if reviewers didn't see any problems with this approach,
your series would already have review tags.
I got that and did not mean to ignore those comments forever.
If you want to suggest a different approach to managing MemoryRegions, then I
would suggest you send a proposal to the mailing list so it can be reviewed
by the QOM and memory people (along with updated code style documentation so
That's what I plan to do but got no time right now.
we can all use it). But with freeze coming up in a few days this is certainly
out of scope for 10.2.
That's why I proposed to take this patch for now as it only moves existing
code to another place so it does not introduce a new problem that's not
already there. And the problem is theoretical as it does not cause a leak
in this case and nobody raised concerns so far and it was only noticed by
this patch making it clearer removing duplicated code. So I still think
this patch has merit, it helped to find a bug and would make code simpler
without making it worse so it could be taken for that. But I'm OK with
dropping it too, I'll include it again in the series with the other
proposed changes to fix the problem.
For now I would suggest the easiest way to get review tags is to stick with
the existing inline MemoryRegion approach for 10.2 to aid getting your
patches merged: it's simply not fair to put time pressure on Harsh to merge
patches in this way.
I'm not putting time pressure on Harsh, the coming freeze does and I don't
have time right now to change patches only later but I don't want to miss
a release again. I've already submitted these pegasos1 emulation series
for the previous release but due to missing maintainer it was missed then.
I think it's now resolved by dropping this patch and taking the rest of
the series which is fine with me. Thanks everybody for the reviews and
help. I'll prepare patches to resolve concerns when I'll have time again.
Regards,
BALATON Zoltan
