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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
