Re: [PATCH v10 3/7] RAS/AMD/ATL: Add unified UMC address translation interface

[Yazen Ghannam <yazen.ghannam@amd.com>]

On Fri, Nov 06, 2026 at 09:56:35AM -0800, Borislav Petkov wrote:
> On Tue, Sep 22, 2026 at 03:23:13PM -0500, Yazen Ghannam wrote:
> > The library exposes a single function for UMC address translation. Adding a
> > new translation would mean adding and plumbing another export. Callers also
> > have no way to request more than one representation of an address in a
> > single call.
> > 
> > Add a single entry point, amd_translate_umc_mca_addr(), that operates
> > entirely through struct atl_err. Expand the struct with a pair of
> > operation bitmaps and the per-operation output fields.
> > 
> > The caller provides the error inputs and requests one or more operations.
> > Each operation is attempted independently. The library sets a validation
> > bit and fills the matching output for each operation that succeeds. The
> > caller must check the validation bit before consuming an output value.
> > 
> > Group the error identifiers (normalized address, socket, and UMC bank ID)
> > into struct atl_umc_addr. Lay it out to match the PRM parameter buffer
> > inputs that every handler shares, and embed it in the buffer. A caller can
> > then build the buffer straight from its own struct atl_err. Move the buffer
> > definition to internal.h so callers outside prm.c can do so.
> > 
> > Route the interface through the always-built RAS core so the library
> > remains optional. The core provides the entry point and a
> > register/unregister pair. The library registers its implementation on load.
> > The entry point does nothing (no validation bits set) when the library is
> > absent.
> > 
> > Start with a single operation, ATL_OP_SPA, that produces the System
> > Physical Address. Try the PRM handler first. Fall back to the native Data
> > Fabric translation if PRM is unavailable and the Data Fabric revision
> > supports it.
> > 
> > Leave the existing amd_convert_umc_mca_addr_to_sys_addr() interface in
> > place for now.
> 
> Please remove text explaining the code.
> 

Okay.

> > Assisted-by: LLM
> 
> Those tags are not needed anymore. Yeah, that was quick:
> 
>   1e035297ffda ("docs: remove Assisted-by references")
> 
> ...
> 

Okay.

> > diff --git a/drivers/ras/amd/atl/umc.c b/drivers/ras/amd/atl/umc.c
> > index befc616d5e8a..91c02ef5dc11 100644
> > --- a/drivers/ras/amd/atl/umc.c
> > +++ b/drivers/ras/amd/atl/umc.c
> > @@ -416,3 +416,45 @@ unsigned long convert_umc_mca_addr_to_sys_addr(struct atl_err *err)
> >  
> >  	return norm_to_sys_addr(socket_id, die_id, coh_st_inst_id, addr);
> >  }
> > +
> > +/*
> > + * Translate a UMC MCA error address into one or more representations as
> > + * requested by the caller.
> > + *
> > + * The caller sets the input values and requests one or more operations
> > + * through @err->requested. @err->valid is cleared on entry. Each operation
> > + * is attempted independently. The corresponding bit in @err->valid is set
> > + * and the related output field is filled for each operation that succeeds.
> > + * The caller must check @err->valid before consuming an output value.
> > + *
> > + * The PRM handlers consume @err->umc_addr directly. Only MI300 needs its
> > + * MCA_ADDR value converted to a normalized address first; see get_addr().
> > + * MI300 platforms provide no PRM handlers, so the conversion is left to the
> > + * native fallback path.
> 
> Hm, ok. Sounds like a whole protocol. Lemme get to the end of this and see
> whether this is not too overengineered for no good reason...
> 
> 
> > + * Registered with the RAS core as the UMC address translator; see
> > + * amd_translate_umc_mca_addr().
> 
> Unnecessary text.
> 

Okay.

> > + */
> > +void amd_atl_umc_translate_addr(struct atl_err *err)
> > +{
> > +	err->socket_id = topology_physical_package_id(err->cpu);
> > +	err->valid     = 0;
> > +
> > +	if (err->requested & ATL_OP_SPA) {
> 
> You can make those operations a number and this then can become a switch-case.
> 
> Should be easier, like a dispatch table.
> 

I had considered that. But I went with the flag idea so that we could do
multiple operations with a single call.

> > +		struct param_buf p_buf = {
> > +			.addr		= err->umc_addr,
> > +			.out_buf	= &err->spa,
> > +		};
> > +
> > +		if (!prm_umc_norm_to_addr(norm_to_sys_guid, &p_buf)) {
> > +			err->valid |= ATL_OP_SPA;
> > +		} else if (!df_cfg.flags.prm_only) {
> > +			unsigned long spa = convert_umc_mca_addr_to_sys_addr(err);
> > +
> > +			if (!IS_ERR_VALUE(spa)) {
> > +				err->spa = spa;
> > +				err->valid |= ATL_OP_SPA;
> > +			}
> > +		}
> > +	}
> > +}
> 
> ...
> 
> > diff --git a/include/linux/ras.h b/include/linux/ras.h
> > index 468941bfe855..eac8cf39ddd5 100644
> > --- a/include/linux/ras.h
> > +++ b/include/linux/ras.h
> > @@ -3,6 +3,8 @@
> >  #define __RAS_H__
> >  
> >  #include <asm/errno.h>
> > +#include <linux/bits.h>
> > +#include <linux/stddef.h>
> >  #include <linux/uuid.h>
> >  #include <linux/cper.h>
> >  
> > @@ -35,10 +37,26 @@ static inline void
> >  log_arm_hw_error(struct cper_sec_proc_arm *err, const u8 sev) { return; }
> >  #endif
> >  
> > +/* Operations requested and completed through amd_translate_umc_mca_addr(). */
> > +#define ATL_OP_SPA		BIT(0)	/* System Physical Address */
> 
> verify_comment_style: WARNING: No tail comments please.
> 
> Please audit your whole set.
> 

Will do.

Thanks,
Yazen