Re: [PATCH v5 23/25] tests: add plugin asserting correctness of discon event's to_pc

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 5/19/25 8:24 AM, Julian Ganz wrote:
> We recently introduced plugin API for the registration of callbacks for
> discontinuity events, specifically for interrupts, exceptions and host
> call events. The callback receives various bits of information,
> including the VCPU index and PCs.
> 
> This change introduces a test plugin asserting the correctness of that
> behaviour in cases where this is possible with reasonable effort. Since
> instruction PCs are recorded at translation blocks translation time and
> a TB may be used in multiple processes running in distinct virtual
> memory, the plugin allows comparing not full addresses but a subset of
> address bits via the `compare-addr-bits` option.
> 
> Signed-off-by: Julian Ganz <neither@nut.email>
> ---
> 
> Pierrick: I did implement the changes you requested, but I did not add
> your Reviewed-By because I felt the changes were not trivial enough to
> not require a new, coarse review.
>

Looks good to me, thanks.

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Regarding the issue with the same tb being mapped at different virtual 
addresses, I'm ok with the current solution of comparing only page bits.

That said, a better solution could be to compare physical addresses when 
a discon is detected (on plugin side), and confirm it's really a 
discontinuity or just a different mapping. With this approach, it's not 
even needed to have a dedicated option, and there would be no false 
positive in the plugin. It's just a suggestion though.

Regards,
Pierrick