Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/38012 )
Change subject: nb/intel/sandybridge: add and use ME stolen memory and lock bit defines ......................................................................
nb/intel/sandybridge: add and use ME stolen memory and lock bit defines
Change-Id: If4663498b10a5eedcc1aa51088b984ecc49ef23e Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/northbridge/intel/sandybridge/finalize.c M src/northbridge/intel/sandybridge/raminit_common.c M src/northbridge/intel/sandybridge/sandybridge.h 3 files changed, 5 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/38012/1
diff --git a/src/northbridge/intel/sandybridge/finalize.c b/src/northbridge/intel/sandybridge/finalize.c index 60e7a74..e07c6c2 100644 --- a/src/northbridge/intel/sandybridge/finalize.c +++ b/src/northbridge/intel/sandybridge/finalize.c @@ -24,7 +24,7 @@ pci_or_config16(PCI_DEV_SNB, GGC, 1 << 0); pci_or_config16(PCI_DEV_SNB, PAVPC, 1 << 2); pci_or_config32(PCI_DEV_SNB, DPR, 1 << 0); - pci_or_config32(PCI_DEV_SNB, MESEG_MASK, 1 << 10); + pci_or_config32(PCI_DEV_SNB, MESEG_MASK, MELCK); pci_or_config32(PCI_DEV_SNB, REMAPBASE, 1 << 0); pci_or_config32(PCI_DEV_SNB, REMAPLIMIT, 1 << 0); pci_or_config32(PCI_DEV_SNB, TOM, 1 << 0); diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 5b388de..4e42c71 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -608,8 +608,8 @@ reg = pci_read_config32(PCI_DEV(0, 0, 0), MESEG_MASK); val = (0x80000 - me_uma_size) & 0xfff; reg = (reg & ~0xfff00000) | (val << 20); - reg = reg | (1 << 10); // set lockbit on ME mem - reg = reg | (1 << 11); // set ME memory enable + reg = reg | ME_STLEN_EN; // set ME memory enable + reg = reg | MELCK; // set lockbit on ME mem printk(BIOS_DEBUG, "PCI(0, 0, 0)[%x] = %x\n", MESEG_MASK, reg); pci_write_config32(PCI_DEV(0, 0, 0), MESEG_MASK, reg); } diff --git a/src/northbridge/intel/sandybridge/sandybridge.h b/src/northbridge/intel/sandybridge/sandybridge.h index 4bb7003..6dcb3ce 100644 --- a/src/northbridge/intel/sandybridge/sandybridge.h +++ b/src/northbridge/intel/sandybridge/sandybridge.h @@ -83,6 +83,8 @@
#define MESEG_BASE 0x70 #define MESEG_MASK 0x78 +#define MELCK (1 << 10) /* ME Range Lock */ +#define ME_STLEN_EN (1 << 11) /* ME Stolen Memory Enable */
#define PAM0 0x80 #define PAM1 0x81
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38012 )
Change subject: nb/intel/sandybridge: add and use ME stolen memory and lock bit defines ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit_common.c:
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... PS1, Line 612: reg = reg | MELCK; // set lockbit on ME mem Just curious of the line swap. I hope the datasheet does not mention that hardware requires a split enable / lock sequence instead of a single write here.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38012 )
Change subject: nb/intel/sandybridge: add and use ME stolen memory and lock bit defines ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/raminit_common.c:
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... PS1, Line 612: reg = reg | MELCK; // set lockbit on ME mem
Just curious of the line swap. […]
The datasheet I have doesn't mention anything about this, so I don't know for sure. Since it is currently done in one write and seems to work, I'm not going to change this for now. Swapping the two lines here doesn't change the actual write in line 614, but makes more sense to me when reading the code. So this is just a cosmetic change.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38012 )
Change subject: nb/intel/sandybridge: add and use ME stolen memory and lock bit defines ......................................................................
Patch Set 1: Code-Review+2
(2 comments)
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/sandybridge.h:
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... PS1, Line 86: MELCK The official name for this is `MESEG_MASK_MELCK_MASK`
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... PS1, Line 87: ME_STLEN_EN The official name for this is `MESEG_MASK_ME_STLEN_EN_MASK`
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38012 )
Change subject: nb/intel/sandybridge: add and use ME stolen memory and lock bit defines ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/sandybridge.h:
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... PS1, Line 86: MELCK
The official name for this is `MESEG_MASK_MELCK_MASK`
I got this name from the official documentation 326765-005
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... PS1, Line 87: ME_STLEN_EN
The official name for this is `MESEG_MASK_ME_STLEN_EN_MASK`
same
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38012 )
Change subject: nb/intel/sandybridge: add and use ME stolen memory and lock bit defines ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... File src/northbridge/intel/sandybridge/sandybridge.h:
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... PS1, Line 86: MELCK
I got this name from the official documentation 326765-005
Ah, MELCK refers to the field. The name I have uses the register name as prefix and `MASK` as suffix.
https://review.coreboot.org/c/coreboot/+/38012/1/src/northbridge/intel/sandy... PS1, Line 87: ME_STLEN_EN
same
Ack
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/38012 )
Change subject: nb/intel/sandybridge: add and use ME stolen memory and lock bit defines ......................................................................
nb/intel/sandybridge: add and use ME stolen memory and lock bit defines
Change-Id: If4663498b10a5eedcc1aa51088b984ecc49ef23e Signed-off-by: Felix Held felix-coreboot@felixheld.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/38012 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/sandybridge/finalize.c M src/northbridge/intel/sandybridge/raminit_common.c M src/northbridge/intel/sandybridge/sandybridge.h 3 files changed, 5 insertions(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/northbridge/intel/sandybridge/finalize.c b/src/northbridge/intel/sandybridge/finalize.c index 60e7a74..e07c6c2 100644 --- a/src/northbridge/intel/sandybridge/finalize.c +++ b/src/northbridge/intel/sandybridge/finalize.c @@ -24,7 +24,7 @@ pci_or_config16(PCI_DEV_SNB, GGC, 1 << 0); pci_or_config16(PCI_DEV_SNB, PAVPC, 1 << 2); pci_or_config32(PCI_DEV_SNB, DPR, 1 << 0); - pci_or_config32(PCI_DEV_SNB, MESEG_MASK, 1 << 10); + pci_or_config32(PCI_DEV_SNB, MESEG_MASK, MELCK); pci_or_config32(PCI_DEV_SNB, REMAPBASE, 1 << 0); pci_or_config32(PCI_DEV_SNB, REMAPLIMIT, 1 << 0); pci_or_config32(PCI_DEV_SNB, TOM, 1 << 0); diff --git a/src/northbridge/intel/sandybridge/raminit_common.c b/src/northbridge/intel/sandybridge/raminit_common.c index 5b388de..4e42c71 100644 --- a/src/northbridge/intel/sandybridge/raminit_common.c +++ b/src/northbridge/intel/sandybridge/raminit_common.c @@ -608,8 +608,8 @@ reg = pci_read_config32(PCI_DEV(0, 0, 0), MESEG_MASK); val = (0x80000 - me_uma_size) & 0xfff; reg = (reg & ~0xfff00000) | (val << 20); - reg = reg | (1 << 10); // set lockbit on ME mem - reg = reg | (1 << 11); // set ME memory enable + reg = reg | ME_STLEN_EN; // set ME memory enable + reg = reg | MELCK; // set lockbit on ME mem printk(BIOS_DEBUG, "PCI(0, 0, 0)[%x] = %x\n", MESEG_MASK, reg); pci_write_config32(PCI_DEV(0, 0, 0), MESEG_MASK, reg); } diff --git a/src/northbridge/intel/sandybridge/sandybridge.h b/src/northbridge/intel/sandybridge/sandybridge.h index 4bb7003..6dcb3ce 100644 --- a/src/northbridge/intel/sandybridge/sandybridge.h +++ b/src/northbridge/intel/sandybridge/sandybridge.h @@ -83,6 +83,8 @@
#define MESEG_BASE 0x70 #define MESEG_MASK 0x78 +#define MELCK (1 << 10) /* ME Range Lock */ +#define ME_STLEN_EN (1 << 11) /* ME Stolen Memory Enable */
#define PAM0 0x80 #define PAM1 0x81