linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Rework MCA configuration handling code, v2
@ 2012-10-25 14:17 Borislav Petkov
  2012-10-25 14:17 ` [PATCH 1/5] drivers/base: Add a DEVICE_BOOL_ATTR macro Borislav Petkov
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Borislav Petkov @ 2012-10-25 14:17 UTC (permalink / raw)
  To: Tony Luck; +Cc: Greg Kroah-Hartman, Naveen N. Rao, LKML, Borislav Petkov

From: Borislav Petkov <bp@alien8.de>

Third round, incorporating feedback from the last time.

Changelog:
==========

v1:
---

this is the first complete series implementing what we discussed
previously. It now uses simple bools and collects everything in a struct
mca_config which is nicely abstracted and collected in one place.

Please take a look and complain :)

v0:
---

Right,

so I did give that a try and it turned out to be a bit more involved
than I thought. Basically, I'm relying on the assumption that if I use a
u64 bitfield and pass a pointer to it, accessing it through that pointer
as a u64 value works. Actually, I have the u64 bitfield as the first
member of a struct and if I cast a pointer to that struct to u64 *, I'm
expecting to have the 64 bits of the same bitfield.

Therefore, I can toggle the bits in the mce code with mca_cfg.<bitname>.
When defining accessing them through the device attributes in sysfs, I
use a new macro DEVICE_BIT_ATTR which gets the corresponding bit number
of that same bit in the bitfield. This gives only one function which
operates on a bitfield instead of a single function per bit in the
bitfield.

For example,

mca_cfg {
	u64 dont_log_ce : 1,

is the first bit in the bitfield and I also have a MCA_CFG_DONT_LOG_CE
define which is 0, i.e. the first bit, which I use to toggle the
corresponding bit in the u64 in device_{show,store}_bit.

So I converted mce_dont_log_ce and mce_disabled (renaming it into the
more correct mca_disabled) and it seems to work.

The asm looks sane too. One other advantage is that it makes the code
much more cleaner and compact by collecting all those bool config values
in a single struct, which was my original incentive to do that.

So please take a look and let me know whether this is sane, especially
the above assumption that I can access a u64 bitfield through a u64 *
and the bits are where they're expected to be. gcc seems to do that...
and I don't see anything in the C99 standard that would object to it but
I could be overlooking something.

Thanks.

Borislav Petkov (5):
  drivers/base: Add a DEVICE_BOOL_ATTR macro
  x86, MCA: Convert dont_log_ce, banks and tolerant
  x86, MCA: Convert rip_msr, mce_bootlog, monarch_timeout
  x86, MCA: Convert the next three variables batch
  x86, MCA: Finish mca_config conversion

 arch/x86/include/asm/mce.h                |  21 ++-
 arch/x86/kernel/cpu/mcheck/mce-internal.h |   2 -
 arch/x86/kernel/cpu/mcheck/mce-severity.c |   4 +-
 arch/x86/kernel/cpu/mcheck/mce.c          | 209 +++++++++++++++---------------
 arch/x86/kernel/cpu/mcheck/mce_intel.c    |   8 +-
 arch/x86/lguest/boot.c                    |   2 +-
 drivers/base/core.c                       |  21 +++
 include/linux/device.h                    |   7 +
 8 files changed, 159 insertions(+), 115 deletions(-)

-- 
1.8.0


^ permalink raw reply	[flat|nested] 17+ messages in thread
* [PATCH 0/5] Rework MCA configuration handling code
@ 2012-10-17 11:13 Borislav Petkov
  2012-10-17 11:13 ` [PATCH 1/5] drivers/base: Add a DEVICE_BOOL_ATTR macro Borislav Petkov
  0 siblings, 1 reply; 17+ messages in thread
From: Borislav Petkov @ 2012-10-17 11:13 UTC (permalink / raw)
  To: Tony Luck; +Cc: Greg Kroah-Hartman, Naveen N. Rao, LKML, Borislav Petkov

From: Borislav Petkov <borislav.petkov@amd.com>

Ok,

this is the first complete series implementing what we discussed
previously. It now uses simple bools and collects everything in a struct
mca_config which is nicely abstracted and collected in one place.

Please take a look and complain :)

Changelog:
==========

v0:
---

Right,

so I did give that a try and it turned out to be a bit more involved
than I thought. Basically, I'm relying on the assumption that if I use a
u64 bitfield and pass a pointer to it, accessing it through that pointer
as a u64 value works. Actually, I have the u64 bitfield as the first
member of a struct and if I cast a pointer to that struct to u64 *, I'm
expecting to have the 64 bits of the same bitfield.

Therefore, I can toggle the bits in the mce code with mca_cfg.<bitname>.
When defining accessing them through the device attributes in sysfs, I
use a new macro DEVICE_BIT_ATTR which gets the corresponding bit number
of that same bit in the bitfield. This gives only one function which
operates on a bitfield instead of a single function per bit in the
bitfield.

For example,

mca_cfg {
	u64 dont_log_ce : 1,

is the first bit in the bitfield and I also have a MCA_CFG_DONT_LOG_CE
define which is 0, i.e. the first bit, which I use to toggle the
corresponding bit in the u64 in device_{show,store}_bit.

So I converted mce_dont_log_ce and mce_disabled (renaming it into the
more correct mca_disabled) and it seems to work.

The asm looks sane too. One other advantage is that it makes the code
much more cleaner and compact by collecting all those bool config values
in a single struct, which was my original incentive to do that.

So please take a look and let me know whether this is sane, especially
the above assumption that I can access a u64 bitfield through a u64 *
and the bits are where they're expected to be. gcc seems to do that...
and I don't see anything in the C99 standard that would object to it but
I could be overlooking something.

Thanks.


--

Borislav Petkov (5):
  drivers/base: Add a DEVICE_BOOL_ATTR macro
  x86, MCA: Convert dont_log_ce, banks and tolerant
  x86, MCA: Convert rip_msr, mce_bootlog, monarch_timeout
  x86, MCA: Convert the next three variables batch
  x86, MCA: Finish mca_config conversion

 arch/x86/include/asm/mce.h                |  21 ++-
 arch/x86/kernel/cpu/mcheck/mce-internal.h |   2 -
 arch/x86/kernel/cpu/mcheck/mce-severity.c |   4 +-
 arch/x86/kernel/cpu/mcheck/mce.c          | 213 +++++++++++++++---------------
 arch/x86/kernel/cpu/mcheck/mce_intel.c    |   8 +-
 arch/x86/lguest/boot.c                    |   2 +-
 drivers/base/core.c                       |  24 ++++
 include/linux/device.h                    |   7 +
 8 files changed, 164 insertions(+), 117 deletions(-)

-- 
1.8.0.rc0.18.gf84667d


^ permalink raw reply	[flat|nested] 17+ messages in thread
end of thread, other threads:[~2012-10-25 18:21 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-25 14:17 [PATCH 0/5] Rework MCA configuration handling code, v2 Borislav Petkov
2012-10-25 14:17 ` [PATCH 1/5] drivers/base: Add a DEVICE_BOOL_ATTR macro Borislav Petkov
2012-10-25 16:07   ` Greg Kroah-Hartman
2012-10-25 16:33     ` Borislav Petkov
2012-10-25 16:43       ` Greg Kroah-Hartman
2012-10-25 14:17 ` [PATCH 2/5] x86, MCA: Convert dont_log_ce, banks and tolerant Borislav Petkov
2012-10-25 14:17 ` [PATCH 3/5] x86, MCA: Convert rip_msr, mce_bootlog, monarch_timeout Borislav Petkov
2012-10-25 14:17 ` [PATCH 4/5] x86, MCA: Convert the next three variables batch Borislav Petkov
2012-10-25 14:17 ` [PATCH 5/5] x86, MCA: Finish mca_config conversion Borislav Petkov
2012-10-25 16:50 ` [PATCH 0/5] Rework MCA configuration handling code, v2 Luck, Tony
2012-10-25 16:52   ` Borislav Petkov
  -- strict thread matches above, loose matches on Subject: below --
2012-10-17 11:13 [PATCH 0/5] Rework MCA configuration handling code Borislav Petkov
2012-10-17 11:13 ` [PATCH 1/5] drivers/base: Add a DEVICE_BOOL_ATTR macro Borislav Petkov
2012-10-17 19:03   ` Greg Kroah-Hartman
2012-10-17 20:10     ` Borislav Petkov
2012-10-17 20:29       ` Greg Kroah-Hartman
2012-10-17 20:36         ` Borislav Petkov
2012-10-17 20:44           ` Greg Kroah-Hartman

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).