Re: [PATCH v4 01/12] xen/arm: gicv3: refactor obtaining GIC addresses for common operations

[Julien Grall <julien@xxxxxxx>]

Hi Leonid,

On 28/08/2025 17:17, Leonid Komarianskyi wrote:
> On 28.08.25 15:00, Julien Grall wrote:
> > Hi Leonid,
> >
> > On 27/08/2025 19:24, Leonid Komarianskyi wrote:
> > > Currently, many common functions perform the same operations to calculate
> > > GIC register addresses. This patch consolidates the similar code into
> > > a separate helper function to improve maintainability and reduce
> > > duplication.
> > > This refactoring also simplifies the implementation of eSPI support in
> > > future
> > > changes.
> > >
> > > Signed-off-by: Leonid Komarianskyi <leonid_komarianskyi@xxxxxxxx>
> > > Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>
> > >
> > > ---
> > > Changes in V4:
> > > - no changes
> > >
> > > Changes in V3:
> > > - changed panic() in get_addr_by_offset() to printing warning and
> > >     ASSERT_UNREACHABLE()
> > > - added verification of return pointer from get_addr_by_offset() in the
> > >     callers
> > > - moved invocation of get_addr_by_offset() from spinlock guards, since
> > >     it is not necessarry
> > > - added RB from Volodymyr Babchuk
> > Procces remark, here you said the Reviewed-by from Volodymyr was added
> > in v3. However, given the changes you made this should have been
> > invalidated (reviewed-by means the person read the code and confirmed it
> > is correct).
> >
> > I see Volodymyr confirmed his reviewed-by on v3. So no issue, but this
> > should have been clarified in the changelog.
> Thank you for your explanation.
> Just to clarify: would it be okay to leave the RB tag (with appropriate
> text in the changelog) if I fix some minor nit from another reviewer in
> the next version, like in this patch?
It depends on the change. In general, typoes or coding style changes (I 
include s/uX/uint_X) are fine to keep the review by. Anything else may 
need a review again.
Acked-by are different because they don't carry a full review. So for 
slightly bigger change it would be fine to keep. But if the logic is 
fully rewritten, then they would need to be dropped.
Cheers,

--
Julien Grall