* [PATCH 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers
@ 2017-03-22 19:29 Yazen Ghannam
2017-03-22 19:29 ` [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration Yazen Ghannam
2017-03-27 17:27 ` [PATCH 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers Borislav Petkov
0 siblings, 2 replies; 10+ messages in thread
From: Yazen Ghannam @ 2017-03-22 19:29 UTC (permalink / raw)
To: linux-edac; +Cc: Yazen Ghannam, Tony Luck, Borislav Petkov, x86, linux-kernel
From: Yazen Ghannam <yazen.ghannam@amd.com>
We have support for the new SMCA MCA_DE{STAT,ADDR} registers in Linux. So
we've used these registers in place of MCA_{STATUS,ADDR} on SMCA systems.
However, the guidance for current implementations of SMCA is to continue
using MCA_{STATUS,ADDR} and to use MCA_DE{STAT,ADDR} only if a Deferred
error was not found in the former registers. This also means we shouldn't
clear MCA_CONFIG[LogDeferredInMcaStat].
Redo the AMD Deferred error interrupt handler to follow the guidance for
current SMCA systems. Also, don't break after finding the first error.
Don't clear MCA_CONFIG[LogDeferredInMcaStat] during AMD mcheck init.
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
arch/x86/kernel/cpu/mcheck/mce_amd.c | 47 ++++++++++++++++--------------------
1 file changed, 21 insertions(+), 26 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 524cc57..4e459e0 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -472,20 +472,6 @@ prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
smca_high |= BIT(0);
/*
- * SMCA logs Deferred Error information in MCA_DE{STAT,ADDR}
- * registers with the option of additionally logging to
- * MCA_{STATUS,ADDR} if MCA_CONFIG[LogDeferredInMcaStat] is set.
- *
- * This bit is usually set by BIOS to retain the old behavior
- * for OSes that don't use the new registers. Linux supports the
- * new registers so let's disable that additional logging here.
- *
- * MCA_CONFIG[LogDeferredInMcaStat] is bit 34 (bit 2 in the high
- * portion of the MSR).
- */
- smca_high &= ~BIT(2);
-
- /*
* SMCA sets the Deferred Error Interrupt type per bank.
*
* MCA_CONFIG[DeferredIntTypeSupported] is bit 5, and tells us
@@ -756,7 +742,8 @@ int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr)
EXPORT_SYMBOL_GPL(umc_normaddr_to_sysaddr);
static void
-__log_error(unsigned int bank, bool deferred_err, bool threshold_err, u64 misc)
+__log_error(unsigned int bank, bool deferred_err, bool use_smca_destat,
+ bool threshold_err, u64 misc)
{
u32 msr_status = msr_ops.status(bank);
u32 msr_addr = msr_ops.addr(bank);
@@ -765,7 +752,7 @@ __log_error(unsigned int bank, bool deferred_err, bool threshold_err, u64 misc)
WARN_ON_ONCE(deferred_err && threshold_err);
- if (deferred_err && mce_flags.smca) {
+ if (deferred_err && use_smca_destat) {
msr_status = MSR_AMD64_SMCA_MCx_DESTAT(bank);
msr_addr = MSR_AMD64_SMCA_MCx_DEADDR(bank);
}
@@ -807,6 +794,10 @@ __log_error(unsigned int bank, bool deferred_err, bool threshold_err, u64 misc)
mce_log(&m);
+ /* We should still clear MCA_DESTAT even if we used MCA_STATUS. */
+ if (mce_flags.smca && !use_smca_destat)
+ wrmsrl(MSR_AMD64_SMCA_MCx_DESTAT(bank), 0);
+
wrmsrl(msr_status, 0);
}
@@ -832,25 +823,29 @@ asmlinkage __visible void __irq_entry smp_trace_deferred_error_interrupt(void)
exiting_ack_irq();
}
+static inline bool check_deferred_status(u64 status)
+{
+ return ((status & MCI_STATUS_VAL) && (status & MCI_STATUS_DEFERRED));
+}
+
/* APIC interrupt handler for deferred errors */
static void amd_deferred_error_interrupt(void)
{
unsigned int bank;
- u32 msr_status;
u64 status;
for (bank = 0; bank < mca_cfg.banks; ++bank) {
- msr_status = (mce_flags.smca) ? MSR_AMD64_SMCA_MCx_DESTAT(bank)
- : msr_ops.status(bank);
+ rdmsrl(msr_ops.status(bank), status);
- rdmsrl(msr_status, status);
+ if (check_deferred_status(status)) {
+ __log_error(bank, true, false, false, 0);
- if (!(status & MCI_STATUS_VAL) ||
- !(status & MCI_STATUS_DEFERRED))
- continue;
+ } else if (mce_flags.smca) {
+ rdmsrl(MSR_AMD64_SMCA_MCx_DESTAT(bank), status);
- __log_error(bank, true, false, 0);
- break;
+ if (check_deferred_status(status))
+ __log_error(bank, true, true, false, 0);
+ }
}
}
@@ -904,7 +899,7 @@ static void amd_threshold_interrupt(void)
return;
log:
- __log_error(bank, false, true, ((u64)high << 32) | low);
+ __log_error(bank, false, false, true, ((u64)high << 32) | low);
/* Reset threshold block after logging error. */
memset(&tr, 0, sizeof(tr));
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration
2017-03-22 19:29 [PATCH 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers Yazen Ghannam
@ 2017-03-22 19:29 ` Yazen Ghannam
2017-03-28 19:23 ` Borislav Petkov
2017-03-27 17:27 ` [PATCH 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers Borislav Petkov
1 sibling, 1 reply; 10+ messages in thread
From: Yazen Ghannam @ 2017-03-22 19:29 UTC (permalink / raw)
To: linux-edac; +Cc: Yazen Ghannam, Tony Luck, Borislav Petkov, x86, linux-kernel
From: Yazen Ghannam <yazen.ghannam@amd.com>
Scalable MCA systems have a new MCA_CONFIG register that we use to
configure each bank. We currently use this when we set up thresholding.
However, this is logically separate.
Move setup of MCA_CONFIG into a separate function.
Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
---
arch/x86/kernel/cpu/mcheck/mce_amd.c | 48 ++++++++++++++++++++----------------
1 file changed, 27 insertions(+), 21 deletions(-)
diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
index 4e459e0..95870b3 100644
--- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
+++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
@@ -433,7 +433,7 @@ prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
int offset, u32 misc_high)
{
unsigned int cpu = smp_processor_id();
- u32 smca_low, smca_high, smca_addr;
+ u32 smca_low, smca_high;
struct threshold_block b;
int new;
@@ -457,7 +457,29 @@ prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
goto set_offset;
}
- smca_addr = MSR_AMD64_SMCA_MCx_CONFIG(bank);
+ /* Gather LVT offset for thresholding: */
+ if (rdmsr_safe(MSR_CU_DEF_ERR, &smca_low, &smca_high))
+ goto out;
+
+ new = (smca_low & SMCA_THR_LVT_OFF) >> 12;
+
+set_offset:
+ offset = setup_APIC_mce_threshold(offset, new);
+
+ if ((offset == new) && (mce_threshold_vector != amd_threshold_interrupt))
+ mce_threshold_vector = amd_threshold_interrupt;
+
+done:
+ mce_threshold_block_init(&b, offset);
+
+out:
+ return offset;
+}
+
+static void set_smca_config(unsigned int bank)
+{
+ u32 smca_low, smca_high;
+ u32 smca_addr = MSR_AMD64_SMCA_MCx_CONFIG(bank);
if (!rdmsr_safe(smca_addr, &smca_low, &smca_high)) {
/*
@@ -487,24 +509,6 @@ prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
wrmsr(smca_addr, smca_low, smca_high);
}
-
- /* Gather LVT offset for thresholding: */
- if (rdmsr_safe(MSR_CU_DEF_ERR, &smca_low, &smca_high))
- goto out;
-
- new = (smca_low & SMCA_THR_LVT_OFF) >> 12;
-
-set_offset:
- offset = setup_APIC_mce_threshold(offset, new);
-
- if ((offset == new) && (mce_threshold_vector != amd_threshold_interrupt))
- mce_threshold_vector = amd_threshold_interrupt;
-
-done:
- mce_threshold_block_init(&b, offset);
-
-out:
- return offset;
}
/* cpu init entry point, called from mce.c with preempt off */
@@ -515,8 +519,10 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
int offset = -1;
for (bank = 0; bank < mca_cfg.banks; ++bank) {
- if (mce_flags.smca)
+ if (mce_flags.smca) {
get_smca_bank_info(bank);
+ set_smca_config(bank);
+ }
for (block = 0; block < NR_BLOCKS; ++block) {
address = get_block_address(cpu, address, low, high, bank, block);
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers
2017-03-22 19:29 [PATCH 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers Yazen Ghannam
2017-03-22 19:29 ` [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration Yazen Ghannam
@ 2017-03-27 17:27 ` Borislav Petkov
2017-04-04 13:30 ` Ghannam, Yazen
1 sibling, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2017-03-27 17:27 UTC (permalink / raw)
To: Yazen Ghannam; +Cc: linux-edac, Tony Luck, x86, linux-kernel
On Wed, Mar 22, 2017 at 02:29:30PM -0500, Yazen Ghannam wrote:
> From: Yazen Ghannam <yazen.ghannam@amd.com>
>
> We have support for the new SMCA MCA_DE{STAT,ADDR} registers in Linux. So
> we've used these registers in place of MCA_{STATUS,ADDR} on SMCA systems.
> However, the guidance for current implementations of SMCA is to continue
> using MCA_{STATUS,ADDR} and to use MCA_DE{STAT,ADDR} only if a Deferred
> error was not found in the former registers. This also means we shouldn't
> clear MCA_CONFIG[LogDeferredInMcaStat].
>
> Redo the AMD Deferred error interrupt handler to follow the guidance for
> current SMCA systems. Also, don't break after finding the first error.
>
> Don't clear MCA_CONFIG[LogDeferredInMcaStat] during AMD mcheck init.
>
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> ---
> arch/x86/kernel/cpu/mcheck/mce_amd.c | 47 ++++++++++++++++--------------------
> 1 file changed, 21 insertions(+), 26 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/mcheck/mce_amd.c b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> index 524cc57..4e459e0 100644
> --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c
> +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c
> @@ -472,20 +472,6 @@ prepare_threshold_block(unsigned int bank, unsigned int block, u32 addr,
> smca_high |= BIT(0);
>
> /*
> - * SMCA logs Deferred Error information in MCA_DE{STAT,ADDR}
> - * registers with the option of additionally logging to
> - * MCA_{STATUS,ADDR} if MCA_CONFIG[LogDeferredInMcaStat] is set.
> - *
> - * This bit is usually set by BIOS to retain the old behavior
> - * for OSes that don't use the new registers. Linux supports the
> - * new registers so let's disable that additional logging here.
> - *
> - * MCA_CONFIG[LogDeferredInMcaStat] is bit 34 (bit 2 in the high
> - * portion of the MSR).
> - */
> - smca_high &= ~BIT(2);
> -
> - /*
> * SMCA sets the Deferred Error Interrupt type per bank.
> *
> * MCA_CONFIG[DeferredIntTypeSupported] is bit 5, and tells us
> @@ -756,7 +742,8 @@ int umc_normaddr_to_sysaddr(u64 norm_addr, u16 nid, u8 umc, u64 *sys_addr)
> EXPORT_SYMBOL_GPL(umc_normaddr_to_sysaddr);
>
> static void
> -__log_error(unsigned int bank, bool deferred_err, bool threshold_err, u64 misc)
> +__log_error(unsigned int bank, bool deferred_err, bool use_smca_destat,
> + bool threshold_err, u64 misc)
So I had to paste the function signature in a separate vim window and
keep looking between those arguments' names and the function calls.
Because if I look at:
__log_error(bank, true, false, false, 0);
I absolutely have no clue what that code does. So we need to think of
something better. From the looks of it, I guess we dealt with a single
__log_error() function long enough. Perhaps it is time for separation:
log_error_deferred
log_error_smca
log_error...
and have a function __log_error() which all three call to do the work
which all three share.
That should make the code readable again IMO. __log_error() as it is now
is hard to follow anyway.
> {
> u32 msr_status = msr_ops.status(bank);
> u32 msr_addr = msr_ops.addr(bank);
> @@ -765,7 +752,7 @@ __log_error(unsigned int bank, bool deferred_err, bool threshold_err, u64 misc)
>
> WARN_ON_ONCE(deferred_err && threshold_err);
>
> - if (deferred_err && mce_flags.smca) {
> + if (deferred_err && use_smca_destat) {
> msr_status = MSR_AMD64_SMCA_MCx_DESTAT(bank);
> msr_addr = MSR_AMD64_SMCA_MCx_DEADDR(bank);
> }
> @@ -807,6 +794,10 @@ __log_error(unsigned int bank, bool deferred_err, bool threshold_err, u64 misc)
>
> mce_log(&m);
>
> + /* We should still clear MCA_DESTAT even if we used MCA_STATUS. */
> + if (mce_flags.smca && !use_smca_destat)
> + wrmsrl(MSR_AMD64_SMCA_MCx_DESTAT(bank), 0);
> +
> wrmsrl(msr_status, 0);
> }
>
> @@ -832,25 +823,29 @@ asmlinkage __visible void __irq_entry smp_trace_deferred_error_interrupt(void)
> exiting_ack_irq();
> }
>
> +static inline bool check_deferred_status(u64 status)
This function name does not tell me anything.
if (is_deferred_error(status))
tells me more.
Thanks.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration
2017-03-22 19:29 ` [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration Yazen Ghannam
@ 2017-03-28 19:23 ` Borislav Petkov
2017-04-04 13:34 ` Ghannam, Yazen
0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2017-03-28 19:23 UTC (permalink / raw)
To: Yazen Ghannam; +Cc: linux-edac, Tony Luck, x86, linux-kernel
On Wed, Mar 22, 2017 at 02:29:31PM -0500, Yazen Ghannam wrote:
> From: Yazen Ghannam <yazen.ghannam@amd.com>
>
> Scalable MCA systems have a new MCA_CONFIG register that we use to
> configure each bank. We currently use this when we set up thresholding.
> However, this is logically separate.
>
> Move setup of MCA_CONFIG into a separate function.
>
> Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> ---
> arch/x86/kernel/cpu/mcheck/mce_amd.c | 48 ++++++++++++++++++++----------------
> 1 file changed, 27 insertions(+), 21 deletions(-)
...
> /* cpu init entry point, called from mce.c with preempt off */
> @@ -515,8 +519,10 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
> int offset = -1;
>
> for (bank = 0; bank < mca_cfg.banks; ++bank) {
> - if (mce_flags.smca)
> + if (mce_flags.smca) {
> get_smca_bank_info(bank);
> + set_smca_config(bank);
Or simply bundle those two which do something SMCA-aware per bank into a single:
smca_configure(bank);
which reads almost like a sentence.
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers
2017-03-27 17:27 ` [PATCH 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers Borislav Petkov
@ 2017-04-04 13:30 ` Ghannam, Yazen
0 siblings, 0 replies; 10+ messages in thread
From: Ghannam, Yazen @ 2017-04-04 13:30 UTC (permalink / raw)
To: Borislav Petkov; +Cc: linux-edac, Tony Luck, x86, linux-kernel
> -----Original Message-----
> From: Borislav Petkov [mailto:bp@suse.de]
> Sent: Monday, March 27, 2017 1:27 PM
> To: Ghannam, Yazen <Yazen.Ghannam@amd.com>
...
> > static void
> > -__log_error(unsigned int bank, bool deferred_err, bool threshold_err,
> > u64 misc)
> > +__log_error(unsigned int bank, bool deferred_err, bool use_smca_destat,
> > + bool threshold_err, u64 misc)
>
> So I had to paste the function signature in a separate vim window and keep
> looking between those arguments' names and the function calls.
>
> Because if I look at:
>
> __log_error(bank, true, false, false, 0);
>
> I absolutely have no clue what that code does. So we need to think of
> something better. From the looks of it, I guess we dealt with a single
> __log_error() function long enough. Perhaps it is time for separation:
>
> log_error_deferred
> log_error_smca
> log_error...
>
> and have a function __log_error() which all three call to do the work which all
> three share.
>
> That should make the code readable again IMO. __log_error() as it is now is
> hard to follow anyway.
>
Okay, will do.
> > @@ -832,25 +823,29 @@ asmlinkage __visible void __irq_entry
> smp_trace_deferred_error_interrupt(void)
> > exiting_ack_irq();
> > }
> >
> > +static inline bool check_deferred_status(u64 status)
>
> This function name does not tell me anything.
>
> if (is_deferred_error(status))
>
> tells me more.
>
Okay.
Thanks,
Yazen
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration
2017-03-28 19:23 ` Borislav Petkov
@ 2017-04-04 13:34 ` Ghannam, Yazen
2017-04-04 13:45 ` Borislav Petkov
0 siblings, 1 reply; 10+ messages in thread
From: Ghannam, Yazen @ 2017-04-04 13:34 UTC (permalink / raw)
To: Borislav Petkov; +Cc: linux-edac, Tony Luck, x86, linux-kernel
> -----Original Message-----
> From: Borislav Petkov [mailto:bp@suse.de]
> Sent: Tuesday, March 28, 2017 3:23 PM
> To: Ghannam, Yazen <Yazen.Ghannam@amd.com>
> Cc: linux-edac@vger.kernel.org; Tony Luck <tony.luck@intel.com>;
> x86@kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration
>
> On Wed, Mar 22, 2017 at 02:29:31PM -0500, Yazen Ghannam wrote:
> > From: Yazen Ghannam <yazen.ghannam@amd.com>
> >
> > Scalable MCA systems have a new MCA_CONFIG register that we use to
> > configure each bank. We currently use this when we set up thresholding.
> > However, this is logically separate.
> >
> > Move setup of MCA_CONFIG into a separate function.
> >
> > Signed-off-by: Yazen Ghannam <yazen.ghannam@amd.com>
> > ---
> > arch/x86/kernel/cpu/mcheck/mce_amd.c | 48
> > ++++++++++++++++++++----------------
> > 1 file changed, 27 insertions(+), 21 deletions(-)
>
> ...
>
> > /* cpu init entry point, called from mce.c with preempt off */ @@
> > -515,8 +519,10 @@ void mce_amd_feature_init(struct cpuinfo_x86 *c)
> > int offset = -1;
> >
> > for (bank = 0; bank < mca_cfg.banks; ++bank) {
> > - if (mce_flags.smca)
> > + if (mce_flags.smca) {
> > get_smca_bank_info(bank);
> > + set_smca_config(bank);
>
> Or simply bundle those two which do something SMCA-aware per bank into a
> single:
>
> smca_configure(bank);
>
> which reads almost like a sentence.
>
I'd like to keep the functions separate since they're logically independent. I can
define something like smca_configure() as a wrapper function that can contain
current and future SMCA related functions. Is this okay?
Thanks,
Yazen
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration
2017-04-04 13:34 ` Ghannam, Yazen
@ 2017-04-04 13:45 ` Borislav Petkov
2017-04-04 14:37 ` Ghannam, Yazen
0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2017-04-04 13:45 UTC (permalink / raw)
To: Ghannam, Yazen; +Cc: linux-edac, Tony Luck, x86, linux-kernel
On Tue, Apr 04, 2017 at 01:34:42PM +0000, Ghannam, Yazen wrote:
> I'd like to keep the functions separate since they're logically independent. I can
> define something like smca_configure() as a wrapper function that can contain
> current and future SMCA related functions. Is this okay?
Are you planning to call the one and not the other in some path?
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration
2017-04-04 13:45 ` Borislav Petkov
@ 2017-04-04 14:37 ` Ghannam, Yazen
2017-04-04 15:00 ` Borislav Petkov
0 siblings, 1 reply; 10+ messages in thread
From: Ghannam, Yazen @ 2017-04-04 14:37 UTC (permalink / raw)
To: Borislav Petkov; +Cc: linux-edac, Tony Luck, x86, linux-kernel
> -----Original Message-----
> From: Borislav Petkov [mailto:bp@alien8.de]
> Sent: Tuesday, April 04, 2017 9:46 AM
> To: Ghannam, Yazen <Yazen.Ghannam@amd.com>
> Cc: linux-edac@vger.kernel.org; Tony Luck <tony.luck@intel.com>;
> x86@kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration
>
> On Tue, Apr 04, 2017 at 01:34:42PM +0000, Ghannam, Yazen wrote:
> > I'd like to keep the functions separate since they're logically
> > independent. I can define something like smca_configure() as a wrapper
> > function that can contain current and future SMCA related functions. Is this
> okay?
>
> Are you planning to call the one and not the other in some path?
No, not at the moment. This patch is just to help with readability.
Thanks,
Yazen
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration
2017-04-04 14:37 ` Ghannam, Yazen
@ 2017-04-04 15:00 ` Borislav Petkov
2017-04-04 15:08 ` Ghannam, Yazen
0 siblings, 1 reply; 10+ messages in thread
From: Borislav Petkov @ 2017-04-04 15:00 UTC (permalink / raw)
To: Ghannam, Yazen; +Cc: linux-edac, Tony Luck, x86, linux-kernel
On Tue, Apr 04, 2017 at 02:37:34PM +0000, Ghannam, Yazen wrote:
> No, not at the moment. This patch is just to help with readability.
Then no need to have two separate functions just because stuff is
logically independent. Add a comment over the SMCA configuration part in
case we want to carve it out later.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration
2017-04-04 15:00 ` Borislav Petkov
@ 2017-04-04 15:08 ` Ghannam, Yazen
0 siblings, 0 replies; 10+ messages in thread
From: Ghannam, Yazen @ 2017-04-04 15:08 UTC (permalink / raw)
To: Borislav Petkov; +Cc: linux-edac, Tony Luck, x86, linux-kernel
> -----Original Message-----
> From: Borislav Petkov [mailto:bp@alien8.de]
> Sent: Tuesday, April 04, 2017 11:01 AM
> To: Ghannam, Yazen <Yazen.Ghannam@amd.com>
> Cc: linux-edac@vger.kernel.org; Tony Luck <tony.luck@intel.com>;
> x86@kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration
>
> On Tue, Apr 04, 2017 at 02:37:34PM +0000, Ghannam, Yazen wrote:
> > No, not at the moment. This patch is just to help with readability.
>
> Then no need to have two separate functions just because stuff is logically
> independent. Add a comment over the SMCA configuration part in case we
> want to carve it out later.
>
Okay, will do.
Thanks,
Yazen
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-04-04 15:08 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-22 19:29 [PATCH 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers Yazen Ghannam
2017-03-22 19:29 ` [PATCH 2/2] x86/mce/AMD: Carve out SMCA bank configuration Yazen Ghannam
2017-03-28 19:23 ` Borislav Petkov
2017-04-04 13:34 ` Ghannam, Yazen
2017-04-04 13:45 ` Borislav Petkov
2017-04-04 14:37 ` Ghannam, Yazen
2017-04-04 15:00 ` Borislav Petkov
2017-04-04 15:08 ` Ghannam, Yazen
2017-03-27 17:27 ` [PATCH 1/2] x86/mce/AMD: Redo use of SMCA MCA_DE{STAT,ADDR} registers Borislav Petkov
2017-04-04 13:30 ` Ghannam, Yazen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).