[edk2-devel] [PATCH] OvmfPkg/XenPlatformPei: Relocate shared_info page mapping

[Laszlo Ersek]

On 06/29/21 13:41, Jan Beulich via groups.io wrote:
> On 29.06.2021 13:29, Laszlo Ersek wrote:
>> On 06/29/21 12:35, Andrew Cooper wrote:
>>> On 29/06/2021 11:07, Jan Beulich wrote:
>>>> On 29.06.2021 11:20, Laszlo Ersek wrote:
>>>>> On 06/28/21 15:23, Anthony PERARD via groups.io wrote:
>>>>>> From: Anthony PERARD <anthony.perard at citrix.com>
>>>>>>
>>>>>> Unfortunately, Xen isn't ready to deal with mapping at the top of the
>>>>>> physical address space, so we relocate the mapping after the LAPIC
>>>>>> location.
>>>>>>
>>>>>> See this thread about the issue with the mapping:
>>>>>> - https://lore.kernel.org/xen-devel/f8c4151a-6dac-d87c-ef46-eb35ada07bd9@suse.com/
>>>>>>
>>>>>> The PhysicalAddressIdentityMapping() call isn't going to do anything
>>>>>> anymore since everything under 4GB is already mapped, but there is no
>>>>>> need to remove the call.
>>>>>>
>>>>>> CC: Jan Beulich <JBeulich at suse.com>
>>>>>> CC: Andrew Cooper <andrew.cooper3 at citrix.com>
>>>>>> Signed-off-by: Anthony PERARD <anthony.perard at citrix.com>
>>>>>> ---
>>>>>>  OvmfPkg/XenPlatformPei/Xen.c | 2 +-
>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c
>>>>>> index a4e82b356936..9c6641895970 100644
>>>>>> --- a/OvmfPkg/XenPlatformPei/Xen.c
>>>>>> +++ b/OvmfPkg/XenPlatformPei/Xen.c
>>>>>> @@ -569,7 +569,7 @@ CalibrateLapicTimer (
>>>>>>    EFI_STATUS            Status;
>>>>>>  
>>>>>>  
>>>>>> -  SharedInfo = (VOID*)((1ULL << mPhysMemAddressWidth) - EFI_PAGE_SIZE);
>>>>>> +  SharedInfo = (VOID*)((UINTN)PcdGet32 (PcdCpuLocalApicBaseAddress) + SIZE_1MB);
>>>>>>    Status = PhysicalAddressIdentityMapping ((EFI_PHYSICAL_ADDRESS)SharedInfo);
>>>>>>    if (EFI_ERROR (Status)) {
>>>>>>      DEBUG ((DEBUG_ERROR,
>>>>>>
>>>>> Acked-by: Laszlo Ersek <lersek at redhat.com>
>>>>>
>>>>> I guess I should merge this after Jan and/or Andrew ack it.
>>>> Well, I can informally ack that a move like this is needed, but I can't
>>>> really give an Acked-by on the change, as I know next to nothing about
>>>> ovmf, and hence I cannot, for example, tell whether the chosen new
>>>> location is okay to use there.
>>>
>>> That bit is easy, although it probably does warrant a code comment.  The
>>> choice of location doesn't matter specifically (so long as it's not too
>>> high for 32bit toolstacks to cope with), but wants to be something which
>>> isn't RAM, and ideally doesn't shatter a superpage.
>>>
>>> The various MMIO ranges (IO-APIC, TPM, LAPIC) are 2M aligned, so LAPIC +
>>> 1M is firmly in the middle of a range not used by anything else, and
>>> with APICV/AVIC, the LAPIC does have a 4k mapping to the sink page.
>>
>> I'm OK with informal ACKs of course, just please let me know when you
>> guys feel it's OK for me to merge the patch. I saw the CCs and so I
>> didn't want to hasten and merge the patch before you could object.
> 
> I think you are fine going ahead as long as from the ovmf side all is
> good here.

Merged as commit b37cfdd28071, via
<https://github.com/tianocore/edk2/pull/1771>.

I had to replace the "CC:" lines with "Cc:" ones, in order to placate
"PatchCheck.py".

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#77276): https://edk2.groups.io/g/devel/message/77276
Mute This Topic: https://groups.io/mt/83844392/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-