Hi,
> In PEI module, it also has such assumption, so we don't pass in the
> HOB for the resolved smbase mem size, because we have avoided the
> possibility of error in the reference pi smm cpu driver.
So you essentially are hoping this will never ever change and hard-code
the 8k in both PEI module and PiSmmCpuDxeSmm. I'd suggest to add a
field to the HOB struct instead. If you want stick to the hardcoded 8k
please add a note saying so to the HOB struct description.
> > > + BufferPages = EFI_SIZE_TO_PAGES (SIZE_32KB + TileSize *
> > (mMaxNumberOfCpus - 1));
> >
> > I think correct is:
> > BufferPages = EFI_SIZE_TO_PAGES(TileSize * mMaxNumberOfCpus);
> >
>
> This is the existing code logic & it's correct, not wrong, I don't change it.
> To understand that, we need understand the algorithm of smbase:
>
> The SIZE_32KB covers the *several* SMI Entry and Save State of CPU 0, while
> TileSize * (mMaxNumberOfCpus - 1) to cover Save State of CPU 1+, not include
> the cpu0, so, it's the mMaxNumberOfCpus - 1.
Ok, there is a longish comment in the source code explaining the tiling
(starting at line 639).
smram is 64k (16 pages), with pages 0-7 being unused, page 8 being the
smi handler, 9-14 unused again, page 15 holding cpu state. Due to the
smi handler having an even page index and the cpu state page having a
odd page index you can use that tiling trick so you need only 8k not 32k
per additional cpu.
I agree, the existing code is correct.
> > > + if ((FamilyId == 4) || (FamilyId == 5)) {
> > > + Buffer = AllocateAlignedCodePages (BufferPages, SIZE_32KB);
> >
> > Does that actually matter still? I'm pretty sure we can safely use
> > "ASSERT(FamilyId > 5)" here. Pentium processors have been built in
> > the last century, predating x64.
>
> This is the existing code logic. I don't change it. If you think we don't
> need it, please file Bugzilla for change.
>
> > Beside that the code is broken for SMP, only cpu0 will get a properly
> > aligned smbase. Not sure penium processors support SMP in the first
> > place though ...
>
> I don't understand why "only cpu0 will get a properly aligned smbase"
When the cpu expects the smbase being 32k-aligned (as the comment in the
code explains) the tiling trick just doesn't work. The whole buffer and
the smbase for cpu0 are properly aligned to 32k, but the smbase for cpu1
is not.
take care,
Gerd
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100166): https://edk2.groups.io/g/devel/message/100166
Mute This Topic: https://groups.io/mt/96932003/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
