diff mbox series

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

Message ID 20240314163527.63321-4-john.allen@amd.com (mailing list archive)
State New
Headers show
Series RAS: ATL: DF 4.5 NP2 Denormalization | expand

Commit Message

John Allen March 14, 2024, 4:35 p.m. UTC
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@amd.com>
---
 drivers/ras/amd/atl/dehash.c   | 2 +-
 drivers/ras/amd/atl/internal.h | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

Comments

Yazen Ghannam March 18, 2024, 3:46 p.m. UTC | #1
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@amd.com>
> ---
>   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.

Thanks,
Yazen
John Allen March 25, 2024, 7:57 p.m. UTC | #2
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@amd.com>
> > ---
> >   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?

Thanks,
John
Yazen Ghannam March 26, 2024, 1:47 p.m. UTC | #3
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@amd.com>
>>> ---
>>>    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
diff mbox series

Patch

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