From: Borislav Petkov <bp@alien8.de>
To: Punit Agrawal <punit1.agrawal@toshiba.co.jp>
Cc: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>,
x86@kernel.org, linux-kernel@vger.kernel.org,
linux-pm@vger.kernel.org, linux-edac@vger.kernel.org,
linux-efi@vger.kernel.org, linux-acpi@vger.kernel.org,
devel@acpica.org, Tony Luck <tony.luck@intel.com>,
"Rafael J . Wysocki" <rjw@rjwysocki.net>,
Len Brown <len.brown@intel.com>, Ard Biesheuvel <ardb@kernel.org>,
Yazen Ghannam <yazen.ghannam@amd.com>
Subject: Re: [PATCH v4] cper, apei, mce: Pass x86 CPER through the MCA handling chain
Date: Wed, 23 Sep 2020 16:05:12 +0200 [thread overview]
Message-ID: <20200923140512.GJ28545@zn.tnic> (raw)
In-Reply-To: <87wo0kiz6y.fsf@kokedama.swc.toshiba.co.jp>
Smita,
pls sync the time of the box where you create the patch:
Date: Fri, 4 Sep 2020 09:04:44 -0500
but your mail headers have:
Received: from ... with mapi id 15.20.3370.019; Fri, 18 Sep 2020 14:49:12 +0000
^^^^^^^^^^^^^^^^^^
On Wed, Sep 23, 2020 at 07:07:17PM +0900, Punit Agrawal wrote:
> I know Boris asked you to add the reason for the Reported-by, but
> usually we don't track version differences in the committed patch.
>
> Boris, can you confirm if you want the Reported-by to be retained?
How else would you explain what the Reported-by: tag is for on a patch
which adds a feature?
> > + * The first expected register in the register layout of MCAX address space.
> > + * The address defined must match with the first MSR address extracted from
> > + * BERT which in SMCA systems is the bank's MCA_STATUS register.
> > + *
> > + * Note that the decoding of the raw MSR values in BERT is implementation
> > + * specific and follows register offset order of MCAX address space.
> > + */
> > +#define MASK_MCA_STATUS 0xC0002001
>
> The macro value is already defined in mce.h as
> MSR_AMD64_SMCA_MC0_STATUS. Is there any reason to not use it?
Good point.
> You can move the comment to where you check the status register.
No need if he really wants to use the first MCi_STATUS address.
> > + m.apicid = lapic_id;
> > + m.bank = (ctx_info->msr_addr >> 4) & 0xFF;
> > + m.status = *i_mce;
> > + m.addr = *(i_mce + 1);
> > + m.misc = *(i_mce + 2);
> > + /* Skipping MCA_CONFIG */
> > + m.ipid = *(i_mce + 4);
> > + m.synd = *(i_mce + 5);
>
> Instead of using the raw pointer arithmetic, it is better to define a
> structure for the MCA registers? Something like -
>
> struct {
> u64 addr;
> u64 misc;
> u64 config;
> u64 ipid;
> ...
> }
>
> Checking back, this was mentioned in the previous review comments as
> well. Please address all comments before posting a new version - either
> by following the suggestion or explaining why it is not a good idea.
Well, that was addressed in his reply last time:
https://lkml.kernel.org/r/a28aa613-8353-0052-31f6-34bc733abf59@amd.com
You might've missed it because you weren't CCed directly.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
next prev parent reply other threads:[~2020-09-23 14:05 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-04 14:04 [PATCH v4] cper, apei, mce: Pass x86 CPER through the MCA handling chain Smita Koralahalli
2020-09-23 10:07 ` Punit Agrawal
2020-09-23 14:05 ` Borislav Petkov [this message]
2020-09-23 14:52 ` Ard Biesheuvel
2020-09-23 15:39 ` Borislav Petkov
2020-09-23 18:24 ` Ard Biesheuvel
2020-09-24 0:02 ` Punit Agrawal
2020-09-24 17:23 ` Smita Koralahalli Channabasappa
2020-09-24 17:50 ` Borislav Petkov
2020-09-25 0:54 ` Punit Agrawal
2020-09-25 7:07 ` Borislav Petkov
2020-09-25 16:19 ` Yazen Ghannam
2020-09-25 16:27 ` Borislav Petkov
2020-09-28 8:06 ` Punit Agrawal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200923140512.GJ28545@zn.tnic \
--to=bp@alien8.de \
--cc=Smita.KoralahalliChannabasappa@amd.com \
--cc=ardb@kernel.org \
--cc=devel@acpica.org \
--cc=len.brown@intel.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-edac@vger.kernel.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=punit1.agrawal@toshiba.co.jp \
--cc=rjw@rjwysocki.net \
--cc=tony.luck@intel.com \
--cc=x86@kernel.org \
--cc=yazen.ghannam@amd.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).