Re: SMRAM sizes on large hosts

[Laszlo Ersek <lersek@redhat.com>]

On 05/03/17 15:44, Gerd Hoffmann wrote:
>   Hi,
>
>> I propose the following: add a new fw_cfg file which communicates how
>> much memory (how many megabytes) the "11b" value in the tseg size
>> register will configure.
>
> I'd prefer to keep fw_cfg out of the picture, and I think we can do it
> without too much problems.
>
> We have a TSEGMB (tseg memory base) register.  qemu doesn't implement
> it (but I think you can write to it and read back what you've
> written).

OVMF already uses this register, for the exact purpose you mention.

(1) OVMF sets the register in SmmAccessPeiEntryPoint(), which is the
entry point of the PEI-phase module that produces PEI_SMM_ACCESS_PPI.

(2) OVMF reads the register back in SmramAccessGetCapabilities(), which
is built into two modules:

- the same PEIM as mentioned above, for producing
PEI_SMM_ACCESS_PPI.GetCapabilities(),

- a similar-purpose DXE driver, for producing
EFI_SMM_ACCESS2_PROTOCOL.GetCapabilities().

Those GetCapabilities() member functions basically return the SMRAM map
to the caller.

> We could make qemu update that register in case "11b" is written to
> the tseg size.  The firmware then can read TSEGMB and figure whenever
> a large tseg is supported and if so how big it is.

Do you mean that QEMU would at once calculate the absolute base of TSEG,
downwards from the top of low RAM, expressed in MB?

Excerpt from (1):

>   //
>   // Set TSEG Memory Base.
>   //
>   PciWrite32 (DRAMC_REGISTER_Q35 (MCH_TSEGMB),
>     (TopOfLowRamMb - FixedPcdGet8 (PcdQ35TsegMbytes)) << MCH_TSEGMB_MB_SHIFT);
>
>   //
>   // Set TSEG size, and disable TSEG visibility outside of SMM. Note that the
>   // T_EN bit has inverse meaning; when T_EN is set, then TSEG visibility is
>   // *restricted* to SMM.
>   //
>   EsmramcVal &= ~(UINT32)MCH_ESMRAMC_TSEG_MASK;
>   EsmramcVal |= FixedPcdGet8 (PcdQ35TsegMbytes) == 8 ? MCH_ESMRAMC_TSEG_8MB :
>                 FixedPcdGet8 (PcdQ35TsegMbytes) == 2 ? MCH_ESMRAMC_TSEG_2MB :
>                 MCH_ESMRAMC_TSEG_1MB;
>   EsmramcVal |= MCH_ESMRAMC_T_EN;
>   PciWrite8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC), EsmramcVal);

I guess I could update this to:
- set MCH_TSEGMB like now, but also remember the written value
- write 11b to MCH_ESMRAMC, and read back MCH_TSEGMB
- if the read back value differs from the written one, use that value
for tseg size going forward, plus write 11b | T_EN to MCH_ESMRAMC
- otherwise, set MCH_ESMRAMC like now.

The problem with this is that I need the TSEG size in another module as
well, namely PlatformPei. The dispatch order between PlatformPei and
SmmAccessPei is unspecified (they have both TRUE for DEPEX). If
PlatformPei gets dispatched second, I really wouldn't want to write to
MCH_ESMRAMC again, just to query MCH_TSEGMB. (I couldn't just read
MCH_TSEGMB because if PlatformPei were dispatched first, then MCH_TSEGMB
would still be unset).

In other words, I wouldn't like to write a register just to receive the
information; I need the information in two places whose relative
ordering is unspecified, and one of those already writes the register in
question, genuinely. I guess I could hack it around with another dynamic
PCD, but that's kind of ugly.

If we invented a read-only, side-effect-free PCI config space register
that gave me this value plain and simple (similarly to how a new fw_cfg
file would do), that would be a lot cleaner for me. I think this would
match your earlier alternative where you wrote "Alternatively we could
add some qemu-specific register". That's my next preference after
fw_cfg.

Thanks!
Laszlo