Marx Wang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate RH vulnerability ......................................................................
soc/intel/apollolake: Enable 2x refresh rate to mitigate RH vulnerability
We must enable 2x refresh rate in order to mitigate Row Hammer vulnerability.
BUG=N/A TEST=run suspend_stress_test with memory check for 2500 cycles.
Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/meminit.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/1
diff --git a/src/soc/intel/apollolake/meminit.c b/src/soc/intel/apollolake/meminit.c index 8601571..9815a40 100644 --- a/src/soc/intel/apollolake/meminit.c +++ b/src/soc/intel/apollolake/meminit.c @@ -68,7 +68,7 @@ cfg->SliceHashMask = 0x9; cfg->InterleavedMode = 2; cfg->ChannelsSlicesEnable = 0; - cfg->MinRefRate2xEnable = 0; + cfg->MinRefRate2xEnable = 1; cfg->DualRankSupportEnable = 1; /* Don't enforce a memory size limit. */ cfg->MemorySizeLimit = 0;
Hello build bot (Jenkins), Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#2).
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate RH vulnerability ......................................................................
soc/intel/apollolake: Enable 2x refresh rate to mitigate RH vulnerability
We must enable 2x refresh rate in order to mitigate Row Hammer vulnerability.
BUG=N/A TEST=run suspend_stress_test with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/meminit.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/2
Attention is currently required from: Marx Wang. Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate RH vulnerability ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2: IMHO, the mitigation should be optional for boards since it is highly depending on the what DRAMs (vendor) are on the board. SOC provides this 2x refresh-rate capability. My suggestion is to tweak the title as Enable 2x refresh rate and probably create a flag for CONFIG_2X_REFRESH_RATE to set this capability on/off. The flag CONFIG_2X_REFRESH_RATE would be set in mainboard.
Attention is currently required from: Gaggery Tsai. Marx Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate RH vulnerability ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
IMHO, the mitigation should be optional for boards since it is highly depending on the what DRAMs (v […]
if it's optional for boards, I suggest to move this option to "devicetree" and let customers to decide if it needs to be enabled.
Attention is currently required from: Marx Wang. Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate RH vulnerability ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
if it's optional for boards, I suggest to move this option to "devicetree" and let customers to deci […]
That works for me.
Attention is currently required from: Marx Wang. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate RH vulnerability ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2: Shouldn’t this be configurable, that means defaulting to the secure mode, but de pendend on some build option for users not needing this protection but focus more on power consumption?
How is this tested?
Attention is currently required from: Paul Menzel. Marx Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate RH vulnerability ......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
Shouldn’t this be configurable, that means defaulting to the secure mode, but de […]
1. Firstly, it's a DRAM issue, not the SoC issue. And the issue is highly depending on what DRAMs (vendor) are on the boards. That's why we plan to provide an option in device tree for users to config. 2. we've done some validations and measured the power consumption. There is no functional regression and no significant difference regarding power consumption.
Attention is currently required from: Marx Wang, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate RH vulnerability ......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/48510/comment/62883b69_8652d643 PS2, Line 7: RH vulnerability Row Hammer
Patchset:
PS2:
- Firstly, it's a DRAM issue, not the SoC issue. […]
I'd suggest adding a `ENABLE_DDR_2X_REFRESH` Kconfig option. Mainboards which are known to have issues with regular refresh rate (be it suspend/resume issues or Row Hammer susceptibility) can then select the `ENABLE_DDR_2X_REFRESH` option or override its default.
Attention is currently required from: Marx Wang, Paul Menzel. Hello Gaggery Tsai, build bot (Jenkins), Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#3).
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Enable 2x refresh rate to mitigate Row Hammer vulnerability
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs.
BUG=N/A TEST=run suspend_stress_test with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/meminit.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/3
Attention is currently required from: Marx Wang, Angel Pons. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate Row Hammer vulnerability ......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
I'd suggest adding a `ENABLE_DDR_2X_REFRESH` Kconfig option. […]
1. But you are changing it on the SoC level here, aren’t you?
I like Angel’s proposal.
The question still remains, what state to default to.
Attention is currently required from: Paul Menzel, Angel Pons. Marx Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate Row Hammer vulnerability ......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
- But you are changing it on the SoC level here, aren’t you? […]
after reviewing the codes, adding Kconfing (ex, "ENABLE_DDR_2X_REFRESH") will be much easier. the change will be like:
- cfg->MinRefRate2xEnable = 0; + + if (IS_ENABLED(CONFIG_ENABLE_DDR_2X_REFRESH)) + cfg->MinRefRate2xEnable = 1; + else + cfg->MinRefRate2xEnable = 0; + +
Attention is currently required from: Marx Wang, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Enable 2x refresh rate to mitigate Row Hammer vulnerability ......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
after reviewing the codes, adding Kconfing (ex, "ENABLE_DDR_2X_REFRESH") will be much easier. […]
It's even simpler:
cfg->MinRefRate2xEnable = CONFIG(ENABLE_DDR_2X_REFRESH);
Attention is currently required from: Marx Wang, Paul Menzel. Hello Gaggery Tsai, build bot (Jenkins), Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#4).
Change subject: soc/intel/apollolake: Provide the option to enable 2x refresh rate to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Provide the option to enable 2x refresh rate to mitigate Row Hammer vulnerability
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs.To enable it, we can select "ENABLE_DDR_2X_REFRESH" option.
BUG=N/A TEST=select "ENABLE_DDR_2X_REFRESH", build the image and run suspend_stress_test with memory check for 2500 cycles.
Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/meminit.c 2 files changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/4
Attention is currently required from: Marx Wang, Paul Menzel. Hello Gaggery Tsai, build bot (Jenkins), Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#5).
Change subject: soc/intel/apollolake: Provide the option to enable 2x refresh rate to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Provide the option to enable 2x refresh rate to mitigate Row Hammer vulnerability
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can select "ENABLE_DDR_2X_REFRESH" option.
BUG=N/A TEST=select "ENABLE_DDR_2X_REFRESH", build the image and run suspend_stress_test with memory check for 2500 cycles.
Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/meminit.c 2 files changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/5
Attention is currently required from: Paul Menzel, Angel Pons. Marx Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable 2x refresh rate to mitigate Row Hammer vulnerability ......................................................................
Patch Set 6:
(1 comment)
Patchset:
PS2:
It's even simpler: […]
good idea.
Attention is currently required from: Marx Wang, Paul Menzel, Angel Pons. Hello Gaggery Tsai, build bot (Jenkins), Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#7).
Change subject: soc/intel/apollolake: Provide the option to enable 2x refresh rate to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Provide the option to enable 2x refresh rate to mitigate Row Hammer vulnerability
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs.To enable it, we can select "ENABLE_DDR_2X_REFRESH" option.
BUG=N/A TEST=select "ENABLE_DDR_2X_REFRESH", build the image and run suspend_stress_test with memory check for 2500 cycles.
Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/meminit.c 2 files changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/7
Attention is currently required from: Marx Wang, Paul Menzel, Angel Pons. Hello Gaggery Tsai, build bot (Jenkins), Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#8).
Change subject: soc/intel/apollolake: Provide the option to enable 2x refresh rate to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Provide the option to enable 2x refresh rate to mitigate Row Hammer vulnerability
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs.To enable it, we can select "ENABLE_GLK_DDR_2X_REFRESH" option.
BUG=N/A TEST=select "ENABLE_GLK_DDR_2X_REFRESH", build the image and run suspend_stress_test with memory check for 2500 cycles.
Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/meminit.c 2 files changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/8
Attention is currently required from: Marx Wang, Paul Menzel, Angel Pons. Hello Gaggery Tsai, build bot (Jenkins), Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#9).
Change subject: soc/intel/apollolake: Provide the option to enable 2x refresh rate to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Provide the option to enable 2x refresh rate to mitigate Row Hammer vulnerability
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs.To enable it, we can select "ENABLE_GLK_DDR_2X_REFRESH" option.
BUG=N/A TEST=select "ENABLE_GLK_DDR_2X_REFRESH", build the image and run suspend_stress_test with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/meminit.c 2 files changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/9
Attention is currently required from: Paul Menzel, Angel Pons. Hello Gaggery Tsai, build bot (Jenkins), Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#10).
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs.To enable it, we can select "ENABLE_GLK_DDR_2X_REFRESH" option.
BUG=N/A TEST=select "ENABLE_GLK_DDR_2X_REFRESH", build the image and run suspend_stress_test with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/meminit.c 2 files changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/10
Attention is currently required from: Marx Wang, Angel Pons. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 10:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/48510/comment/3c5ed9a0_feefc493 PS10, Line 10: DRAMs.To Please add a space after the period/dot.
File src/soc/intel/apollolake/Kconfig:
https://review.coreboot.org/c/coreboot/+/48510/comment/3cd41481_34966842 PS10, Line 23: DDR 2x refresh support Please extend the text to also mention Row Hammer.
Attention is currently required from: Marx Wang, Angel Pons. Hello Gaggery Tsai, build bot (Jenkins), Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#11).
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can select "ENABLE_GLK_DDR_2X_REFRESH" option.
BUG=N/A TEST=select "ENABLE_GLK_DDR_2X_REFRESH", build the image and run suspend_stress_test with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/meminit.c 2 files changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/11
Attention is currently required from: Paul Menzel, Angel Pons. Marx Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 11:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/48510/comment/21c2eecc_9a4c92c4 PS10, Line 10: DRAMs.To
Please add a space after the period/dot.
Done
File src/soc/intel/apollolake/Kconfig:
https://review.coreboot.org/c/coreboot/+/48510/comment/91255e26_93f70b99 PS10, Line 23: DDR 2x refresh support
Please extend the text to also mention Row Hammer.
Done
Attention is currently required from: Marx Wang, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 11:
(1 comment)
File src/soc/intel/apollolake/Kconfig:
https://review.coreboot.org/c/coreboot/+/48510/comment/5b7a2493_4c84423a PS11, Line 19: GLK Huh, why add GLK? I guess it's because of this Kconfig error?
Error: Default for 'ENABLE_DDR_2X_REFRESH' referenced at src/northbridge/intel/haswell/Kconfig:105 will never be set - overridden by default set at src/soc/intel/apollolake/Kconfig:21
Simply move the config inside the `if SOC_INTEL_APOLLOLAKE` block, after `config CPU_SPECIFIC_OPTIONS`.
Attention is currently required from: Paul Menzel, Angel Pons. Marx Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 11:
(1 comment)
File src/soc/intel/apollolake/Kconfig:
https://review.coreboot.org/c/coreboot/+/48510/comment/526256df_7a09b0b6 PS11, Line 19: GLK
Huh, why add GLK? I guess it's because of this Kconfig error? […]
yes, you're right.
Attention is currently required from: Paul Menzel, Angel Pons. Hello Gaggery Tsai, build bot (Jenkins), Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#12).
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can select "ENABLE_DDR_2X_REFRESH" option.
BUG=b:152151369 TEST=select "ENABLE_DDR_2X_REFRESH", build the image and run suspend_stress_test with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/meminit.c 2 files changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/12
Attention is currently required from: Paul Menzel, Angel Pons. Hello Gaggery Tsai, build bot (Jenkins), Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#13).
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can select "ENABLE_DDR_2X_REFRESH" option.
BUG=b:152151369 TEST=select "ENABLE_DDR_2X_REFRESH", build the image and run suspend_stress_test with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/meminit.c 2 files changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/13
Attention is currently required from: Marx Wang, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 13: Code-Review+2
Attention is currently required from: Marx Wang, Paul Menzel. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 13:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/48510/comment/31d5c8d9_be6a7193 PS2, Line 7: RH vulnerability
Row Hammer
Done
Attention is currently required from: Marx Wang. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 13: Code-Review+1
Marx Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 13:
(1 comment)
Commit Message:
https://review.coreboot.org/c/coreboot/+/48510/comment/46fa0f13_fe11a92b PS2, Line 7: RH vulnerability
Done
Attention is currently required from: Marx Wang. Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 13: Code-Review-1
(1 comment)
Patchset:
PS13: Should we wait a little bit? It looks to me the CONFIG would impact all DRAMs. Since the mitigation is for DRAM, does it make sense to add an attribute in lpddr4_sku like "bool row_hammer_mitigation"?
So in mainboard, just an example, we could initialize the variable if this DRAM is impacted like [0] = { .speed = LP4_SPEED_2400, .ch0_rank_density = LP4_8Gb_DENSITY, .ch1_rank_density = LP4_8Gb_DENSITY, .ch0_dual_rank = 1, .ch1_dual_rank = 1, .part_num = "K4F6E304HB-MGCJ", .row_hammer_mitigation = CONFIG(ENABLE_2X_MITIGATION) }, /* K4F8E304HB-MGCJ - both logical channels */ .....
This setting will be passed to set_lpddr4_defaults with the slightly change to the meminit_lpddr4 with sku->row_hammer_mitigation.
Attention is currently required from: Marx Wang. Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 13:
(1 comment)
Patchset:
PS13:
Should we wait a little bit? It looks to me the CONFIG would impact all DRAMs. […]
Typo: the mitigation is for some DRAMs.
Attention is currently required from: Gaggery Tsai, Marx Wang. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 13: Code-Review+1
(1 comment)
Patchset:
PS13:
Typo: the mitigation is for some DRAMs.
Oh, good point. I missed that the changed code was only touching LPDDR4.
Attention is currently required from: Gaggery Tsai, Angel Pons. Marx Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 13:
(1 comment)
Patchset:
PS13:
Oh, good point. I missed that the changed code was only touching LPDDR4.
good suggestion since the board will use different DRAMs. To enable it according to DRAM vendors/SKUs is more specific.
Attention is currently required from: Marx Wang, Angel Pons. Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 13:
(1 comment)
Patchset:
PS13: This should work and I don't mind if you pass the sku pointer to meminit_lpddr4.
diff --git a/src/soc/intel/apollolake/include/soc/meminit.h b/src/soc/intel/apollolake/include/soc/meminit.h index 83cf809453..1ae91fbe3a 100644 --- a/src/soc/intel/apollolake/include/soc/meminit.h +++ b/src/soc/intel/apollolake/include/soc/meminit.h @@ -84,7 +84,7 @@ struct lpddr4_swizzle_cfg { * Initialize default LPDDR4 settings with provided speed. No logical channels * are enabled. Subsequent calls to logical channel enabling are required. */ -void meminit_lpddr4(FSP_M_CONFIG *cfg, int speed); +void meminit_lpddr4(FSP_M_CONFIG *cfg, int speed, int rh_mitigation);
/* * Enable logical channel providing the full lpddr4_swizzle_config to @@ -104,6 +104,7 @@ struct lpddr4_sku { int ch1_dual_rank; const char *part_num; bool disable_periodic_retraining; + bool row_hammer_mitigation; };
struct lpddr4_cfg { diff --git a/src/soc/intel/apollolake/meminit.c b/src/soc/intel/apollolake/meminit.c index 86015715bc..115ed6845e 100644 --- a/src/soc/intel/apollolake/meminit.c +++ b/src/soc/intel/apollolake/meminit.c @@ -55,7 +55,7 @@ size_t iohole_in_mib(void) return 2 * (GiB / MiB); }
-static void set_lpddr4_defaults(FSP_M_CONFIG *cfg) +static void set_lpddr4_defaults(FSP_M_CONFIG *cfg, int rh_mitigation) { uint8_t odt_config;
@@ -68,7 +68,7 @@ static void set_lpddr4_defaults(FSP_M_CONFIG *cfg) cfg->SliceHashMask = 0x9; cfg->InterleavedMode = 2; cfg->ChannelsSlicesEnable = 0; - cfg->MinRefRate2xEnable = 0; + cfg->MinRefRate2xEnable = rh_mitigation; cfg->DualRankSupportEnable = 1; /* Don't enforce a memory size limit. */ cfg->MemorySizeLimit = 0; @@ -188,14 +188,14 @@ static int fsp_memory_profile(int speed) return -1; }
-void meminit_lpddr4(FSP_M_CONFIG *cfg, int speed) +void meminit_lpddr4(FSP_M_CONFIG *cfg, int speed, int rh_mitigation) { speed = validate_speed(speed);
printk(BIOS_INFO, "LP4DDR speed is %dMHz\n", speed); cfg->Profile = fsp_memory_profile(speed);
- set_lpddr4_defaults(cfg); + set_lpddr4_defaults(cfg, rh_mitigation); }
static void enable_logical_chan0(FSP_M_CONFIG *cfg, @@ -327,7 +327,7 @@ void meminit_lpddr4_by_sku(FSP_M_CONFIG *cfg,
sku = &lpcfg->skus[sku_id];
- meminit_lpddr4(cfg, sku->speed); + meminit_lpddr4(cfg, sku->speed, sku->row_hammer_mitigation);
if (sku->ch0_rank_density) { printk(BIOS_INFO, "LPDDR4 Ch0 density = %d\n",
Attention is currently required from: Marx Wang, Angel Pons. Hello Gaggery Tsai, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#14).
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can select "ENABLE_DDR_2X_REFRESH" option and add "enable_row_hammer_mitigation" attribute in board's lpddr4 memory config. for example:
static const struct lpddr4_sku cbi_skus[] = { /* Dual Channel Config 4GiB System Capacity */ ............. ................................ /* Dual Channel Config 8GiB System Capacity */ [1] = { .speed = LP4_SPEED_2400, .ch0_rank_density = LP4_16Gb_DENSITY, .ch1_rank_density = LP4_16Gb_DENSITY, .enable_row_hammer_mitigation = CONFIG(ENABLE_DDR_2X_REFRESH), },
BUG=b:152151369 TEST=select "ENABLE_DDR_2X_REFRESH", build the image and run suspend_stress_test with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/include/soc/meminit.h M src/soc/intel/apollolake/meminit.c 3 files changed, 16 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/14
Attention is currently required from: Marx Wang, Angel Pons. Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 14:
(1 comment)
File src/soc/intel/apollolake/Kconfig:
https://review.coreboot.org/c/coreboot/+/48510/comment/47322b4c_0d704ae6 PS14, Line 431: ENABLE_DDR_2X_REFRESH I guess we don't need this since the rh_mitigation flag is passed from mainboard memory setup already.
Attention is currently required from: Gaggery Tsai, Marx Wang. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 14: Code-Review+1
(1 comment)
File src/soc/intel/apollolake/Kconfig:
https://review.coreboot.org/c/coreboot/+/48510/comment/9117224a_6e7ffb6c PS14, Line 431: ENABLE_DDR_2X_REFRESH
I guess we don't need this since the rh_mitigation flag is passed from mainboard memory setup alread […]
Yeah, I agree. At first I thought that the patch enabled 2x refresh for everything, but it's no longer the case. Please also update the commit message.
Attention is currently required from: Gaggery Tsai, Marx Wang. Hello Gaggery Tsai, build bot (Jenkins), Paul Menzel, Angel Pons, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#15).
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate
We need to enable 2x refresh rate in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can add "enable_row_hammer_mitigation" attribute in board's lpddr4 memory config. for example:
static const struct lpddr4_sku cbi_skus[] = { /* Dual Channel Config 4GiB System Capacity */ ............. ................................ /* Dual Channel Config 8GiB System Capacity */ [1] = { .speed = LP4_SPEED_2400, .ch0_rank_density = LP4_16Gb_DENSITY, .ch1_rank_density = LP4_16Gb_DENSITY, .enable_row_hammer_mitigation = 1, },
BUG=b:152151369 TEST=add "enable_row_hammer_mitigation" attribute for specific DRAMs, build the image and run "suspend_stress_test" with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/include/soc/meminit.h M src/soc/intel/apollolake/meminit.c 2 files changed, 10 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/15
Attention is currently required from: Marx Wang. Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 15: Code-Review+2
Attention is currently required from: Gaggery Tsai, Marx Wang, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 15:
(2 comments)
Patchset:
PS15: Before this gets merged, please push the mainboard code that makes use of this for review.
File src/soc/intel/apollolake/Kconfig:
https://review.coreboot.org/c/coreboot/+/48510/comment/c324a20d_dbd563aa PS14, Line 431: ENABLE_DDR_2X_REFRESH
Yeah, I agree. […]
But without this the user has no choice?
Has the Row-Hammer issue become worse? What I remember from my research a few years back, the affected systems still work fine. Even if they are attackable, somebody who runs only trusted software on their system would never need such mitigations, or would they?
Attention is currently required from: Marx Wang, Nico Huber, Angel Pons, Kane Chen. Gaggery Tsai has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 15:
(1 comment)
File src/soc/intel/apollolake/Kconfig:
https://review.coreboot.org/c/coreboot/+/48510/comment/60535999_1ecdff52 PS14, Line 431: ENABLE_DDR_2X_REFRESH
But without this the user has no choice? […]
It's tight to DRAM. IMHO, I think there is a choice here, board vendor could set enable_row_hammer_mitigation = 0 or leave it as default in case no fear for the attack. Please let me know if I got your message wrong.
Attention is currently required from: Gaggery Tsai, Marx Wang, Nico Huber, Angel Pons, Kane Chen. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48510 )
Change subject: soc/intel/apollolake: Provide the option to enable DDR 2x refresh rate ......................................................................
Patch Set 15:
(1 comment)
File src/soc/intel/apollolake/Kconfig:
https://review.coreboot.org/c/coreboot/+/48510/comment/98fea07e_3063181d PS14, Line 431: ENABLE_DDR_2X_REFRESH
It's tight to DRAM. […]
The point is that even on a device where the mitigation "should" be enabled due to hardware concerns, a (sophisticated) device owner should be able to disable the mitigation without rewriting lots of code (e.g. by setting a build time flag).
Nico's example is that the owner might know that the device is used in a trusted environment only - but really any other reason is valid: they own the box, they're in charge. If they break the computer, they get to keep the pieces (and this issue isn't even about breaking the device).
Attention is currently required from: Gaggery Tsai, Marx Wang, Nico Huber, Angel Pons, Kane Chen. Hello Gaggery Tsai, build bot (Jenkins), Paul Menzel, Angel Pons, Kane Chen, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#16).
Change subject: soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability
We need to enable 2x refresh rate and low watermark in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can add "enable_row_hammer_mitigation" attribute in board's lpddr4 memory config. for example:
static const struct lpddr4_sku cbi_skus[] = { /* Dual Channel Config 4GiB System Capacity */ ............. ................................ /* Dual Channel Config 8GiB System Capacity */ [1] = { .speed = LP4_SPEED_2400, .ch0_rank_density = LP4_16Gb_DENSITY, .ch1_rank_density = LP4_16Gb_DENSITY, .enable_row_hammer_mitigation = 1, },
BUG=b:152151369 TEST=add "enable_row_hammer_mitigation" attribute for specific DRAMs, build the image and run "suspend_stress_test" with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/include/soc/meminit.h M src/soc/intel/apollolake/meminit.c M src/vendorcode/intel/fsp/fsp2_0/glk/FspmUpd.h 3 files changed, 34 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/16
Attention is currently required from: Gaggery Tsai, Nico Huber, Angel Pons, Kane Chen. Hello Gaggery Tsai, build bot (Jenkins), Paul Menzel, Angel Pons, Kane Chen, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#17).
Change subject: soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability
We need to enable 2x refresh rate and low watermark in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can add "enable_row_hammer_mitigation" attribute in board's lpddr4 memory config. for example:
static const struct lpddr4_sku cbi_skus[] = { /* Dual Channel Config 4GiB System Capacity */ ............. ................................ /* Dual Channel Config 8GiB System Capacity */ [1] = { .speed = LP4_SPEED_2400, .ch0_rank_density = LP4_16Gb_DENSITY, .ch1_rank_density = LP4_16Gb_DENSITY, .enable_row_hammer_mitigation = 1, },
CQ-DEPEND=chrome-internal:3456385 BUG=b:152151369 TEST=add "enable_row_hammer_mitigation" attribute for specific DRAMs, build the image and run "suspend_stress_test" with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/include/soc/meminit.h M src/soc/intel/apollolake/meminit.c M src/vendorcode/intel/fsp/fsp2_0/glk/FspmUpd.h 3 files changed, 34 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/17
Attention is currently required from: Marx Wang, Gaggery Tsai, Nico Huber, Angel Pons, Kane Chen. Hello Gaggery Tsai, build bot (Jenkins), Paul Menzel, Angel Pons, Kane Chen, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#18).
Change subject: soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability
We can enable 2x refresh rate and low watermark in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can add "enable_row_hammer_mitigation" attribute in board's lpddr4 memory config. for example:
static const struct lpddr4_sku cbi_skus[] = { /* Dual Channel Config 4GiB System Capacity */ ............. ................................ /* Dual Channel Config 8GiB System Capacity */ [1] = { .speed = LP4_SPEED_2400, .ch0_rank_density = LP4_16Gb_DENSITY, .ch1_rank_density = LP4_16Gb_DENSITY, .enable_row_hammer_mitigation = 1, },
CQ-DEPEND=chrome-internal:3456385 BUG=b:152151369 TEST=add "enable_row_hammer_mitigation" attribute for specific DRAMs, build the image and run "suspend_stress_test" with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/include/soc/meminit.h M src/soc/intel/apollolake/meminit.c M src/vendorcode/intel/fsp/fsp2_0/glk/FspmUpd.h 3 files changed, 34 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/18
Attention is currently required from: Marx Wang, Gaggery Tsai, Nico Huber, Angel Pons, Kane Chen. Hello Gaggery Tsai, build bot (Jenkins), Paul Menzel, Angel Pons, Kane Chen, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#19).
Change subject: soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability
We can enable 2x refresh rate and low watermark in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can add "enable_row_hammer_mitigation" attribute in board's lpddr4 memory config. for example:
static const struct lpddr4_sku cbi_skus[] = { /* Dual Channel Config 4GiB System Capacity */ ............. ................................ /* Dual Channel Config 8GiB System Capacity */ [1] = { .speed = LP4_SPEED_2400, .ch0_rank_density = LP4_16Gb_DENSITY, .ch1_rank_density = LP4_16Gb_DENSITY, .enable_row_hammer_mitigation = 1, },
CQ-DEPEND=chrome-internal:3456385 BUG=b:152151369 TEST=add "enable_row_hammer_mitigation=1" attribute for specific DRAMs, build the image and run "suspend_stress_test" with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/include/soc/meminit.h M src/soc/intel/apollolake/meminit.c M src/vendorcode/intel/fsp/fsp2_0/glk/FspmUpd.h 3 files changed, 34 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/19
Attention is currently required from: Marx Wang, Gaggery Tsai, Nico Huber, Angel Pons, Kane Chen. Hello Gaggery Tsai, build bot (Jenkins), Paul Menzel, Angel Pons, Kane Chen, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#21).
Change subject: soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability
We need to enable 2x refresh rate and low watermark in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can add "enable_row_hammer_mitigation" attribute in board's lpddr4 memory config. for example:
static const struct lpddr4_sku cbi_skus[] = { /* Dual Channel Config 4GiB System Capacity */ ............. ................................ /* Dual Channel Config 8GiB System Capacity */ [1] = { .speed = LP4_SPEED_2400, .ch0_rank_density = LP4_16Gb_DENSITY, .ch1_rank_density = LP4_16Gb_DENSITY, .enable_row_hammer_mitigation = 1, },
BUG=b:152151369 TEST=add "enable_row_hammer_mitigation" attribute for specific DRAMs, build the image and run "suspend_stress_test" with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/include/soc/meminit.h M src/soc/intel/apollolake/meminit.c M src/vendorcode/intel/fsp/fsp2_0/glk/FspmUpd.h 3 files changed, 36 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/21
Attention is currently required from: Gaggery Tsai, Nico Huber, Angel Pons, Kane Chen. Hello Gaggery Tsai, build bot (Jenkins), Paul Menzel, Angel Pons, Kane Chen, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48510
to look at the new patch set (#23).
Change subject: soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability ......................................................................
soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability
We need to enable 2x refresh rate and low watermark in order to mitigate Row Hammer vulnerability for the boards with specific DRAMs. To enable it, we can add "enable_row_hammer_mitigation" attribute in board's lpddr4 memory config. for example:
static const struct lpddr4_sku cbi_skus[] = { /* Dual Channel Config 4GiB System Capacity */ ............. ................................ /* Dual Channel Config 8GiB System Capacity */ [1] = { .speed = LP4_SPEED_2400, .ch0_rank_density = LP4_16Gb_DENSITY, .ch1_rank_density = LP4_16Gb_DENSITY, .enable_row_hammer_mitigation = 1, }, CQ-DEPEND=chrome-internal:3456385 BUG=b:152151369 TEST=add "enable_row_hammer_mitigation" attribute for specific DRAMs, build the image and run "suspend_stress_test" with memory check for 2500 cycles.
Signed-off-by: Marx Wang marx.wang@intel.com Change-Id: I0235fc7906626f28f14367c283433e5b066cc89a --- M src/soc/intel/apollolake/include/soc/meminit.h M src/soc/intel/apollolake/meminit.c M src/vendorcode/intel/fsp/fsp2_0/glk/FspmUpd.h 3 files changed, 36 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/10/48510/23
Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/48510?usp=email )
Change subject: soc/intel/apollolake: Provide the option to mitigate Row Hammer vulnerability ......................................................................
Abandoned
This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.