On 19 February 2016 at 13:33, Laszlo Ersek <ler...@redhat.com> wrote:
> On 02/19/16 13:18, Ard Biesheuvel wrote:
>> On 19 February 2016 at 13:14, Laszlo Ersek <ler...@redhat.com> wrote:
>>> On 02/19/16 12:52, Ard Biesheuvel wrote:
>>>> On 19 February 2016 at 10:56, Laszlo Ersek <ler...@redhat.com> wrote:
>>>>> On 02/19/16 09:53, Ard Biesheuvel wrote:
>>>>>> On 19 February 2016 at 09:45, Yao, Jiewen <jiewen....@intel.com> wrote:
>>>>>>> I can explain the reason on allocating <4G. It is because this data
>>>>>>> will be used in PEI phase in S3 resume.
>>>>>>>
>>>>>>> On most X86 platform, PEI phase is 32bit, and DXE phase is 32bit or
>>>>>>> 64bit. So we have to limit the allocation <4G.
>>>>>>>
>>>>>>
>>>>>> OK, got it.
>>>>>>
>>>>>>> Using ACPI version PCD looks strange here. Maybe another PCD, like
>>>>>>> PcdDxeIplSwitchToLongMode?
>>>>>>>
>>>>>>
>>>>>> But that does not tell us if we are running a 32-bit PEI, right? It
>>>>>> only tells us if a 32-bit PEI should load a 32-bit DXE core on a
>>>>>> 64-bit capable machine.
>>>>>
>>>>> 32->32 and 64->64 builds have PcdDxeIplSwitchToLongMode set to FALSE.
>>>>>
>>>>> Only 32->64 builds have PcdDxeIplSwitchToLongMode set to TRUE.
>>>>>
>>>>> (This is what we have in the three DSC files of OVMF.)
>>>>>
>>>>> So, in order to see in DXE if your PEI is 32-bit, you can do:
>>>>>
>>>>> BOOLEAN PeiIs32Bit;
>>>>>
>>>>> PeiIs32Bit = FALSE;
>>>>> if (FeaturePcdGet (PcdDxeIplSwitchToLongMode)) {
>>>>> //
>>>>> // this implies a 32->64 switch
>>>>> //
>>>>> PeiIs32Bit = TRUE;
>>>>> } else {
>>>>> //
>>>>> // otherwise, PEI and DXE have the same bitness,
>>>>> // so derive it from DXE's bitness
>>>>> //
>>>>> #if defined (MDE_CPU_IA32) || defined (MDE_CPU_ARM)
>>>>> PeiIs32Bit = TRUE;
>>>>> #endif
>>>>> }
>>>>>
>>>>> Alternatively:
>>>>>
>>>>> PeiIs32Bit = FeaturePcdGet (PcdDxeIplSwitchToLongMode) ||
>>>>> sizeof (UINTN) == sizeof (UINT32);
>>>>>
>>>>> I'll admit I'm not really sure about EBC. The above mostly treats EBC as
>>>>> nonexistent. :)
>>>>>
>>>>
>>>> Thanks for clarifying.
>>>>
>>>> So the only case where we have to take care to allocate below 4 GB is
>>>> the 32->64 case, since in all other cases, the memory allocated in DXE
>>>> will always be addressable in PEI.
>>>
>>> I don't think so. PEI is not obliged to be able to address all of the
>>> memory.
>>>
>>> In OVMF for example, even if PEI is 64-bit, the reset vector builds page
>>> tables only for the first 4GB (for the SEC and PEI phases), and only the
>>> DXE core identify-maps the full memory.
>>>
>>
>> OK, so that would imply that we simply need to check for
>> defined(MDE_CPU_X64) here? Or does the same apply to IPF?
>>
>> Since this is a bit of a can of worms, I am perfectly happy to only
>> drop the limit for MDE_CPU_AARCH64 instead, and make everybody's lives
>> easier ...
>
> Okay, so the new PCD idea has been raised several times. IIRC Jiewen
> mentioned that proliferation of PCDs is now frowned upon. On the other
> hand, in this thread, Jiewen mentioned the possibility of a new PCD:
>
> http://thread.gmane.org/gmane.comp.bios.edk2.devel/7817/focus=7871
>
> I think customizing this code for MDE_CPU_AARCH64, on the source code
> level, is not good.
>
> Here we are under IntelFrameworkModulePkg/Universal/BdsDxe/, which has
> two consequences:
>
> - it says Universal, so it shouldn't be specific to architecture on the
> source code level,
> - it says IntelFrameworkModulePkg, so you have a PCD namespace that
> doesn't clutter MdePkg's / MdeModulePkg's.
>
> Also, you mentioned earlier that memory layout is not specific to ISA,
> but platform, hence MDE_CPU_AARCH64 would not be a perfect match anyway.
>
> My vote is to introduce a new PCD that controls this allocation limit.
> After all, it can depend on a bunch of things:
> - Whether you have S3 or not
> - whether PEI can address all of the memory or just 4GB
> - whether your platform has RAM under 4GB
> - whether allocating under 4GB is detrimental to performance
>
> and so on. I think this is the textbook case for a new PCD.
>
OK, fair enough. That still means a PCD with different default values
for X64 and other archs, but that is apparently not something to care
about.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
