[BUG] nb/intel/i945: smm_subregion() prints assert when it should not

[Petr Cvek]

Hello,

On i945 northbridge:

ASSERTION ERROR: file 'src/cpu/x86/smm/tseg_region.c', line 31

can be triggered by some configurations. The assert

ASSERT(IS_ALIGNED(sub_base, sub_size));

tests alignment of SMM base with SMM size. The problem is that IGD stolen memory can offset the SMM base by a smaller size than the SMM size. This causes SMM base to be unaligned. For example:

80000000: 2 GiB Top of RAM, stored in TOLUD
7ff00000: 1 MiB Stolen IGD RAM base, address in BSM
7fd00000: 2 MiB SMM base, defined as SMM size in ESMRAMC

-> 0x7fd00000 is not aligned to SMM size. Probably works for different settings too.

RFC: should the assert from

https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region....

be removed, changed or ignored on i945? The correct test should be probably for 1 MiB alignement as the base of stolen IGD memory have [31:20] bits. But I understand the smm_subregion() test is a generic one.

regards,
Petr

[Arthur Heymans]

Hi

You want to align tseg for when smrr is supported by the CPU. I would just
drop support for 1MB stolen memory.

Arthur

On Sun, 24 Apr 2022, 05:40 Petr Cvek,  wrote:
> Hello,
>
> On i945 northbridge:
>     ASSERTION ERROR: file 'src/cpu/x86/smm/tseg_region.c', line 31
>
> can be triggered by some configurations. The assert
>     ASSERT(IS_ALIGNED(sub_base, sub_size));
>
> tests alignment of SMM base with SMM size. The problem is that IGD stolen
> memory can offset the SMM base by a smaller size than the SMM size. This
> causes SMM base to be unaligned. For example:
>
> 80000000: 2 GiB Top of RAM, stored in TOLUD
> 7ff00000: 1 MiB Stolen IGD RAM base, address in BSM
> 7fd00000: 2 MiB SMM base, defined as SMM size in ESMRAMC
>
> -> 0x7fd00000 is not aligned to SMM size. Probably works for different
> settings too.
>
> RFC: should the assert from
>
> https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region....
>
> be removed, changed or ignored on i945? The correct test should be
> probably for 1 MiB alignement as the base of stolen IGD memory have [31:20]
> bits. But I understand the smm_subregion() test is a generic one.
>
> regards,
> Petr
> _______________________________________________
> coreboot mailing list -- [e-mail]
> To unsubscribe send an email to [e-mail]

[Petr Cvek]

Thanks! I would never find the SMRR feature.

Hmm that means 4MB stolen memory should be dropped along 1MB (TSEG can up to 8MB on i945). Or alternatively to check if TSEG >= UMA in cmos configuration. I suppose cmos.layout would need to support some kind of scripting though.

Petr

Dne 24. 04. 22 v 18:56 Arthur Heymans napsal(a):
> Hi
>
> You want to align tseg for when smrr is supported by the CPU. I would just drop support for 1MB stolen memory.
>
> Arthur
>
> On Sun, 24 Apr 2022, 05:40 Petr Cvek, <[e-mail] > wrote:
> Hello,
>
> On i945 northbridge:
>
>         ASSERTION ERROR: file 'src/cpu/x86/smm/tseg_region.c', line 31
>
> can be triggered by some configurations. The assert
>
>         ASSERT(IS_ALIGNED(sub_base, sub_size));
>
> tests alignment of SMM base with SMM size. The problem is that IGD stolen memory can offset the SMM base by a smaller size than the SMM size. This causes SMM base to be unaligned. For example:
>
> 80000000: 2 GiB Top of RAM, stored in TOLUD
> 7ff00000: 1 MiB Stolen IGD RAM base, address in BSM
> 7fd00000: 2 MiB SMM base, defined as SMM size in ESMRAMC
>
> -> 0x7fd00000 is not aligned to SMM size. Probably works for different settings too.
>
> RFC: should the assert from
>
>         https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31 <https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31>
>
> be removed, changed or ignored on i945? The correct test should be probably for 1 MiB alignement as the base of stolen IGD memory have [31:20] bits. But I understand the smm_subregion() test is a generic one.
>
> regards,
> Petr
> _______________________________________________
> coreboot mailing list -- [e-mail] <mailto:[e-mail]>
> To unsubscribe send an email to [e-mail] <mailto:[e-mail]>

[Arthur Heymans]

Hi

4MB IGD stolen memory is fine for 2M alignment on TSEG, so only dropping 1M
would be needed.
Also that value goes up to 64MB, not 8M, but it's not documented in the
official documentation.

Kind regards,
Arthur

On Sun, Apr 24, 2022 at 11:54 PM Petr Cvek  wrote:
> Thanks! I would never find the SMRR feature.
>
> Hmm that means 4MB stolen memory should be dropped along 1MB (TSEG can up
> to 8MB on i945). Or alternatively to check if TSEG >= UMA in cmos
> configuration. I suppose cmos.layout would need to support some kind of
> scripting though.
>
> Petr
>
> Dne 24. 04. 22 v 18:56 Arthur Heymans napsal(a):
> > Hi
> >
> > You want to align tseg for when smrr is supported by the CPU. I would
>
> just drop support for 1MB stolen memory.
> > Arthur
> >
> > On Sun, 24 Apr 2022, 05:40 Petr Cvek, <[e-mail] <mailto:
>
> [e-mail]>> wrote:
> > Hello,
> >
> > On i945 northbridge:
> >
> >         ASSERTION ERROR: file 'src/cpu/x86/smm/tseg_region.c', line
>
> 31
> > can be triggered by some configurations. The assert
> >
> >         ASSERT(IS_ALIGNED(sub_base, sub_size));
> >
> > tests alignment of SMM base with SMM size. The problem is that IGD
>
> stolen memory can offset the SMM base by a smaller size than the SMM size.
> This causes SMM base to be unaligned. For example:
> > 80000000: 2 GiB Top of RAM, stored in TOLUD
> > 7ff00000: 1 MiB Stolen IGD RAM base, address in BSM
> > 7fd00000: 2 MiB SMM base, defined as SMM size in ESMRAMC
> >
> > -> 0x7fd00000 is not aligned to SMM size. Probably works for
>
> different settings too.
> > RFC: should the assert from
>
> https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region....
> <
> https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region....
> > be removed, changed or ignored on i945? The correct test should be
>
> probably for 1 MiB alignement as the base of stolen IGD memory have [31:20]
> bits. But I understand the smm_subregion() test is a generic one.
> > regards,
> > Petr
> > _______________________________________________
> > coreboot mailing list -- [e-mail] <mailto:
>
> [e-mail]>
> > To unsubscribe send an email to [e-mail] <mailto:
>
> [e-mail]>
> >

[Petr Cvek]

Dne 25. 04. 22 v 7:51 Arthur Heymans napsal(a):
> 4MB IGD stolen memory is fine for 2M alignment on TSEG, so only dropping 1M would be needed.

Yeah but TSEG can have 8MB and will require 8MB alignment (however I don't know if this setting is used in coreboot).

Petr
> Also that value goes up to 64MB, not 8M, but it's not documented in the official documentation.
>
> Kind regards,
> Arthur
>
> On Sun, Apr 24, 2022 at 11:54 PM Petr Cvek <[e-mail] > wrote:
> Thanks! I would never find the SMRR feature.
>
> Hmm that means 4MB stolen memory should be dropped along 1MB (TSEG can up to 8MB on i945). Or alternatively to check if TSEG >= UMA in cmos configuration. I suppose cmos.layout would need to support some kind of scripting though.
>
> Petr
>
> Dne 24. 04. 22 v 18:56 Arthur Heymans napsal(a):
> > Hi
> >
> > You want to align tseg for when smrr is supported by the CPU. I would just drop support for 1MB stolen memory.
> >
> > Arthur
> >
> > On Sun, 24 Apr 2022, 05:40 Petr Cvek, <[e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>>> wrote:
> >
> >     Hello,
> >
> >     On i945 northbridge:
> >
> >             ASSERTION ERROR: file 'src/cpu/x86/smm/tseg_region.c', line 31
> >
> >     can be triggered by some configurations. The assert
> >
> >             ASSERT(IS_ALIGNED(sub_base, sub_size));
> >
> >     tests alignment of SMM base with SMM size. The problem is that IGD stolen memory can offset the SMM base by a smaller size than the SMM size. This causes SMM base to be unaligned. For example:
> >
> >     80000000: 2 GiB Top of RAM, stored in TOLUD
> >     7ff00000: 1 MiB Stolen IGD RAM base, address in BSM
> >     7fd00000: 2 MiB SMM base, defined as SMM size in ESMRAMC
> >
> >     -> 0x7fd00000 is not aligned to SMM size. Probably works for different settings too.
> >
> >     RFC: should the assert from
> >
> >             https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31 <https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31> <https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31 <https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31>>
> >
> >     be removed, changed or ignored on i945? The correct test should be probably for 1 MiB alignement as the base of stolen IGD memory have [31:20] bits. But I understand the smm_subregion() test is a generic one.
> >
> >     regards,
> >     Petr
> >     _______________________________________________
> >     coreboot mailing list -- [e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>>
> >     To unsubscribe send an email to [e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>>
> >

[Arthur Heymans]

Hi

2M TSEG is hardcoded in nb/intel/i945/early_init.c so it should never have
an 8M value.

On Mon, Apr 25, 2022 at 8:22 AM Petr Cvek  wrote:
> Dne 25. 04. 22 v 7:51 Arthur Heymans napsal(a):
> > 4MB IGD stolen memory is fine for 2M alignment on TSEG, so only dropping
>
> 1M would be needed.
>
> Yeah but TSEG can have 8MB and will require 8MB alignment (however I don't
> know if this setting is used in coreboot).
>
> Petr
> > Also that value goes up to 64MB, not 8M, but it's not documented in the
>
> official documentation.
> > Kind regards,
> > Arthur
> >
> > On Sun, Apr 24, 2022 at 11:54 PM Petr Cvek <[e-mail]
>
> > wrote:
> > Thanks! I would never find the SMRR feature.
> >
> > Hmm that means 4MB stolen memory should be dropped along 1MB (TSEG
>
> can up to 8MB on i945). Or alternatively to check if TSEG >= UMA in cmos
> configuration. I suppose cmos.layout would need to support some kind of
> scripting though.
> > Petr
> >
> > Dne 24. 04. 22 v 18:56 Arthur Heymans napsal(a):
> > > Hi
> > >
> > > You want to align tseg for when smrr is supported by the CPU. I
>
> would just drop support for 1MB stolen memory.
> > >
> > > Arthur
> > >
> > > On Sun, 24 Apr 2022, 05:40 Petr Cvek, <[e-mail]
>
>  <mailto:[e-mail] >> wrote:
> > >
> > >     Hello,
> > >
> > >     On i945 northbridge:
> > >
> > >             ASSERTION ERROR: file 'src/cpu/x86/smm/tseg_region.c',
>
> line 31
> > >
> > >     can be triggered by some configurations. The assert
> > >
> > >             ASSERT(IS_ALIGNED(sub_base, sub_size));
> > >
> > >     tests alignment of SMM base with SMM size. The problem is that
>
> IGD stolen memory can offset the SMM base by a smaller size than the SMM
> size. This causes SMM base to be unaligned. For example:
> > >
> > >     80000000: 2 GiB Top of RAM, stored in TOLUD
> > >     7ff00000: 1 MiB Stolen IGD RAM base, address in BSM
> > >     7fd00000: 2 MiB SMM base, defined as SMM size in ESMRAMC
> > >
> > >     -> 0x7fd00000 is not aligned to SMM size. Probably works for
>
> different settings too.
> > >
> > >     RFC: should the assert from
> > >
> > >
>
> https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region....
> <
> https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region....
> <
> https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region....
> <
> https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region....
> > >
> > >
> > >     be removed, changed or ignored on i945? The correct test
>
> should be probably for 1 MiB alignement as the base of stolen IGD memory
> have [31:20] bits. But I understand the smm_subregion() test is a generic
> one.
> > >
> > >     regards,
> > >     Petr
> > >     _______________________________________________
> > >     coreboot mailing list -- [e-mail] <mailto:
>
> [e-mail]> <mailto:[e-mail] >
> > >     To unsubscribe send an email to [e-mail]
>
>  <mailto:[e-mail]
> >
> > >

[Petr Cvek]

OK thanks for info

Petr

Dne 25. 04. 22 v 8:30 Arthur Heymans napsal(a):
> Hi
>
> 2M TSEG is hardcoded in nb/intel/i945/early_init.c so it should never have an 8M value.
>
> On Mon, Apr 25, 2022 at 8:22 AM Petr Cvek <[e-mail] > wrote:
> Dne 25. 04. 22 v 7:51 Arthur Heymans napsal(a):
> > 4MB IGD stolen memory is fine for 2M alignment on TSEG, so only dropping 1M would be needed.
>
> Yeah but TSEG can have 8MB and will require 8MB alignment (however I don't know if this setting is used in coreboot).
>
> Petr
>
> > Also that value goes up to 64MB, not 8M, but it's not documented in the official documentation.
> >
> > Kind regards,
> > Arthur
> >
> > On Sun, Apr 24, 2022 at 11:54 PM Petr Cvek <[e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>>> wrote:
> >
> >     Thanks! I would never find the SMRR feature.
> >
> >     Hmm that means 4MB stolen memory should be dropped along 1MB (TSEG can up to 8MB on i945). Or alternatively to check if TSEG >= UMA in cmos configuration. I suppose cmos.layout would need to support some kind of scripting though.
> >
> >     Petr
> >
> >     Dne 24. 04. 22 v 18:56 Arthur Heymans napsal(a):
> >     > Hi
> >     >
> >     > You want to align tseg for when smrr is supported by the CPU. I would just drop support for 1MB stolen memory.
> >     >
> >     > Arthur
> >     >
> >     > On Sun, 24 Apr 2022, 05:40 Petr Cvek, <[e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>> <mailto:[e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>>>> wrote:
> >     >
> >     >     Hello,
> >     >
> >     >     On i945 northbridge:
> >     >
> >     >             ASSERTION ERROR: file 'src/cpu/x86/smm/tseg_region.c', line 31
> >     >
> >     >     can be triggered by some configurations. The assert
> >     >
> >     >             ASSERT(IS_ALIGNED(sub_base, sub_size));
> >     >
> >     >     tests alignment of SMM base with SMM size. The problem is that IGD stolen memory can offset the SMM base by a smaller size than the SMM size. This causes SMM base to be unaligned. For example:
> >     >
> >     >     80000000: 2 GiB Top of RAM, stored in TOLUD
> >     >     7ff00000: 1 MiB Stolen IGD RAM base, address in BSM
> >     >     7fd00000: 2 MiB SMM base, defined as SMM size in ESMRAMC
> >     >
> >     >     -> 0x7fd00000 is not aligned to SMM size. Probably works for different settings too.
> >     >
> >     >     RFC: should the assert from
> >     >
> >     >             https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31 <https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31> <https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31 <https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31>> <https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31 <https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31> <https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31 <https://elixir.bootlin.com/coreboot/4.16/source/src/cpu/x86/smm/tseg_region.c#L31>>>
> >     >
> >     >     be removed, changed or ignored on i945? The correct test should be probably for 1 MiB alignement as the base of stolen IGD memory have [31:20] bits. But I understand the smm_subregion() test is a generic one.
> >     >
> >     >     regards,
> >     >     Petr
> >     >     _______________________________________________
> >     >     coreboot mailing list -- [e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>> <mailto:[e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>>>
> >     >     To unsubscribe send an email to [e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>> <mailto:[e-mail] <mailto:[e-mail]> <mailto:[e-mail] <mailto:[e-mail]>>>
> >     >
> >