* [PATCH] OvmfPkg/XenPlatformPei: Relocate shared_info page mapping @ 2021-06-28 13:23 Anthony PERARD 2021-06-29 9:20 ` [edk2-devel] " Laszlo Ersek 0 siblings, 1 reply; 7+ messages in thread From: Anthony PERARD @ 2021-06-28 13:23 UTC (permalink / raw) To: devel Cc: Laszlo Ersek, Ard Biesheuvel, Anthony PERARD, Jan Beulich, Andrew Cooper From: Anthony PERARD <anthony.perard@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@suse.com> CC: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Anthony PERARD <anthony.perard@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, -- Anthony PERARD ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] OvmfPkg/XenPlatformPei: Relocate shared_info page mapping 2021-06-28 13:23 [PATCH] OvmfPkg/XenPlatformPei: Relocate shared_info page mapping Anthony PERARD @ 2021-06-29 9:20 ` Laszlo Ersek 2021-06-29 10:07 ` Jan Beulich 0 siblings, 1 reply; 7+ messages in thread From: Laszlo Ersek @ 2021-06-29 9:20 UTC (permalink / raw) To: devel, anthony.perard; +Cc: Ard Biesheuvel, Jan Beulich, Andrew Cooper On 06/28/21 15:23, Anthony PERARD via groups.io wrote: > From: Anthony PERARD <anthony.perard@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@suse.com> > CC: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Anthony PERARD <anthony.perard@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@redhat.com> I guess I should merge this after Jan and/or Andrew ack it. Thanks Laszlo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] OvmfPkg/XenPlatformPei: Relocate shared_info page mapping 2021-06-29 9:20 ` [edk2-devel] " Laszlo Ersek @ 2021-06-29 10:07 ` Jan Beulich 2021-06-29 10:35 ` Andrew Cooper 0 siblings, 1 reply; 7+ messages in thread From: Jan Beulich @ 2021-06-29 10:07 UTC (permalink / raw) To: Laszlo Ersek; +Cc: Ard Biesheuvel, Andrew Cooper, devel, anthony.perard 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@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@suse.com> >> CC: Andrew Cooper <andrew.cooper3@citrix.com> >> Signed-off-by: Anthony PERARD <anthony.perard@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@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. Jan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] OvmfPkg/XenPlatformPei: Relocate shared_info page mapping 2021-06-29 10:07 ` Jan Beulich @ 2021-06-29 10:35 ` Andrew Cooper 2021-06-29 11:29 ` Laszlo Ersek 0 siblings, 1 reply; 7+ messages in thread From: Andrew Cooper @ 2021-06-29 10:35 UTC (permalink / raw) To: Jan Beulich, Laszlo Ersek; +Cc: Ard Biesheuvel, devel, anthony.perard 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@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@suse.com> >>> CC: Andrew Cooper <andrew.cooper3@citrix.com> >>> Signed-off-by: Anthony PERARD <anthony.perard@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@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. ~Andrew ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] OvmfPkg/XenPlatformPei: Relocate shared_info page mapping 2021-06-29 10:35 ` Andrew Cooper @ 2021-06-29 11:29 ` Laszlo Ersek 2021-06-29 11:41 ` Jan Beulich 0 siblings, 1 reply; 7+ messages in thread From: Laszlo Ersek @ 2021-06-29 11:29 UTC (permalink / raw) To: Andrew Cooper, Jan Beulich; +Cc: Ard Biesheuvel, devel, anthony.perard 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@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@suse.com> >>>> CC: Andrew Cooper <andrew.cooper3@citrix.com> >>>> Signed-off-by: Anthony PERARD <anthony.perard@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@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. Thanks Laszlo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] OvmfPkg/XenPlatformPei: Relocate shared_info page mapping 2021-06-29 11:29 ` Laszlo Ersek @ 2021-06-29 11:41 ` Jan Beulich 2021-06-29 14:43 ` Laszlo Ersek 0 siblings, 1 reply; 7+ messages in thread From: Jan Beulich @ 2021-06-29 11:41 UTC (permalink / raw) To: Laszlo Ersek; +Cc: Ard Biesheuvel, devel, anthony.perard, Andrew Cooper 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@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@suse.com> >>>>> CC: Andrew Cooper <andrew.cooper3@citrix.com> >>>>> Signed-off-by: Anthony PERARD <anthony.perard@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@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. Jan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH] OvmfPkg/XenPlatformPei: Relocate shared_info page mapping 2021-06-29 11:41 ` Jan Beulich @ 2021-06-29 14:43 ` Laszlo Ersek 0 siblings, 0 replies; 7+ messages in thread From: Laszlo Ersek @ 2021-06-29 14:43 UTC (permalink / raw) To: devel, anthony.perard; +Cc: jbeulich, Ard Biesheuvel, Andrew Cooper 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@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@suse.com> >>>>>> CC: Andrew Cooper <andrew.cooper3@citrix.com> >>>>>> Signed-off-by: Anthony PERARD <anthony.perard@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@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-06-29 14:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-06-28 13:23 [PATCH] OvmfPkg/XenPlatformPei: Relocate shared_info page mapping Anthony PERARD 2021-06-29 9:20 ` [edk2-devel] " Laszlo Ersek 2021-06-29 10:07 ` Jan Beulich 2021-06-29 10:35 ` Andrew Cooper 2021-06-29 11:29 ` Laszlo Ersek 2021-06-29 11:41 ` Jan Beulich 2021-06-29 14:43 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox