From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web09.9230.1624977838000537333 for ; Tue, 29 Jun 2021 07:43:58 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=WxahaNHz; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1624977837; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=NaJ5b9Euc6CW0p4qW0KBBIiyu/tgkVMvazr61NcYukQ=; b=WxahaNHzCsHHNrnylJgdpvSoovnWDFsENbgTk2NaD9TEiJDaZrIW2rMLZd02zZ8t84ROjM /5sgs5QqD66R9iwIVJv3QWH1JuseMbtDvJbv3CspCXMQVLl2HWfrk5ZuSqO4nLvfbTuLh7 wiq2wfnyV+M6mUs6ZfWLwCHHPEnaSIY= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-252-epdCtjdkNeSin4R3Jq7lEQ-1; Tue, 29 Jun 2021 10:43:54 -0400 X-MC-Unique: epdCtjdkNeSin4R3Jq7lEQ-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8DC81100C611; Tue, 29 Jun 2021 14:43:53 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-158.ams2.redhat.com [10.36.114.158]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 27B4760854; Tue, 29 Jun 2021 14:43:51 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] OvmfPkg/XenPlatformPei: Relocate shared_info page mapping To: devel@edk2.groups.io, anthony.perard@citrix.com Cc: jbeulich@suse.com, Ard Biesheuvel , Andrew Cooper References: <20210628132337.46345-1-anthony.perard@citrix.com> <4d4af77a-8d70-d473-179f-14b3cbc0e6fd@redhat.com> <4960ad34-2097-0f08-553c-477e90e19fe2@citrix.com> <24915ac4-7892-cd27-f587-a59dcee9f5b7@suse.com> From: "Laszlo Ersek" Message-ID: <623c4db5-016a-4bd8-0fb1-f060850f845a@redhat.com> Date: Tue, 29 Jun 2021 16:43:50 +0200 MIME-Version: 1.0 In-Reply-To: <24915ac4-7892-cd27-f587-a59dcee9f5b7@suse.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit 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 >>>>>> >>>>>> 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 >>>>>> CC: Andrew Cooper >>>>>> Signed-off-by: Anthony PERARD >>>>>> --- >>>>>> 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 >>>>> >>>>> 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 . I had to replace the "CC:" lines with "Cc:" ones, in order to placate "PatchCheck.py". Thanks Laszlo