Re: [PATCH 3/4] target/ppc: Move common check in machne check handlers to a function

[BALATON Zoltan]

On Fri, 23 Jun 2023, Fabiano Rosas wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > From: BALATON Zoltan <balaton@eik.bme.hu>
> >
> > All powerpc exception handlers share some code when handling machine
> > check exceptions. Move this to a common function.
>
> Maybe Machine Check is simple enough, but this kind of sharing of code
> has historically caused pain when people want to change something for
> the modern cpus and end up affecting the old cpus by mistake.
>
> There is also the inverse scenario where someone has access to the old
> HW and just want to make an one-off contribution, but the community gets
> insecure about it because it could also affect the new cpus.

It's a trade off between making these independent and avoiding code 
duplication. I think if we have common things that are the same between 
different CPU models then we should not duplicate those, otherwise they 
can diverge when a problem is fixed in one copy but not in the others. We 
just have to remember that if in the future a new CPU defines these 
differently then we mayy need to add another function implementing that 
for the new CPU but the old CPUs can still share the same code without 
duplicating it. So the question is if this machine check is something that 
is the same across different CPU models or do we ever need to model this 
differently for different CPU models? At least the current implementation 
had the same code duplicated everywhere which this patch resolves.

Regards,
BALATON Zoltan

> Then comes the obvious "solution" which is to bring in an artificial
> identifier (excp. model) to be able to have conditional code inside the
> common function. And that causes problems because no one really knows
> how it maps to actual hardware/ISA.
>
> No objection, just a little cautionary tale. =)