Re: [PATCH 3/4] RAS: ATL: Add map_bits_valid to header

[Yazen Ghannam]

On 3/25/24 15:57, John Allen wrote:
> On Mon, Mar 18, 2024 at 11:46:39AM -0400, Yazen Ghannam wrote:
> > On 3/14/24 12:35, John Allen wrote:
> > > Make map_bits_valid available in the AMD ATL internal header as the
> > > function can be used in other parts of the library.
> > >
> > > Signed-off-by: John Allen <john.allen@xxxxxxx>
> > > ---
> > >    drivers/ras/amd/atl/dehash.c   | 2 +-
> > >    drivers/ras/amd/atl/internal.h | 3 +++
> > >    2 files changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/ras/amd/atl/dehash.c b/drivers/ras/amd/atl/dehash.c
> > > index 4ea46262c4f5..a20cf615b83a 100644
> > > --- a/drivers/ras/amd/atl/dehash.c
> > > +++ b/drivers/ras/amd/atl/dehash.c
> > > @@ -19,7 +19,7 @@
> > >     * If @num_intlv_dies and/or @num_intlv_sockets are 1, it means the
> > >     * respective interleaving is disabled.
> > >     */
> > > -static inline bool map_bits_valid(struct addr_ctx *ctx, u8 bit1, u8 bit2,
> > > +inline bool map_bits_valid(struct addr_ctx *ctx, u8 bit1, u8 bit2,
> > >    				  u8 num_intlv_dies, u8 num_intlv_sockets)
> > >    {
> > >    	if (!(ctx->map.intlv_bit_pos == bit1 || ctx->map.intlv_bit_pos == bit2)) {
> > > diff --git a/drivers/ras/amd/atl/internal.h b/drivers/ras/amd/atl/internal.h
> > > index 05b870fcb24e..4681449321de 100644
> > > --- a/drivers/ras/amd/atl/internal.h
> > > +++ b/drivers/ras/amd/atl/internal.h
> > > @@ -239,6 +239,9 @@ unsigned long convert_umc_mca_addr_to_sys_addr(struct atl_err *err);
> > >    u64 add_base_and_hole(struct addr_ctx *ctx, u64 addr);
> > >    u64 remove_base_and_hole(struct addr_ctx *ctx, u64 addr);
> > > +inline bool map_bits_valid(struct addr_ctx *ctx, u8 bit1, u8 bit2,
> > > +			   u8 num_intlv_dies, u8 num_intlv_sockets);
> > > +
> > >    /*
> > >     * Make a gap in @data that is @num_bits long starting at @bit_num.
> > >     * e.g. data		= 11111111'b
> >
> > Ultimately, the maps should be validated as soon as they are gathered. I
> > figured we would do that later. But that would wipe out this change.
> > And, after looking at dehash.c again, map_bits_valid() isn't used in too
> > many places right now.
> >
> > So I think validate_address_map() from the following patch should be
> > done for all modes first. That way we don't need to add and then remove
> > this function from the header.
>
> I'm not sure I understand. Are you saying that we should just move the
> map_bits_valid function to map.c and then make the map_bits_valid calls
> that are currently in dehash.c to validate_address_map?

I mean that the maps should be validated once as soon as they are
gathered (in map.c). This would happen before we get to dehash
functions. So we don't need any map valid checks in dehash.c.

Thanks,
Yazen