From: "Ni, Ruiyu" <ruiyu.ni@Intel.com>
To: Jian J Wang <jian.j.wang@intel.com>, edk2-devel@lists.01.org
Cc: Michael D Kinney <michael.d.kinney@intel.com>,
Liming Gao <liming.gao@intel.com>
Subject: Re: [PATCH 2/3] IntelFrameworkModulePkg/LegacyBiosDxe: Use macro to enable/disable page 0
Date: Wed, 6 Dec 2017 17:32:28 +0800 [thread overview]
Message-ID: <8504df95-2c80-bc7b-c928-084021649a90@Intel.com> (raw)
In-Reply-To: <20171206073120.2248-3-jian.j.wang@intel.com>
On 12/6/2017 3:31 PM, Jian J Wang wrote:
> Current implementation uses following two methods
>
> EnableNullDetection()
> DisableNullDetection()
>
> to enable/disable page 0. These two methods will check PCD
> PcdNullPointerDetectionPropertyMask to know if the page 0 is disabled or not.
> This is due to the fact that old GCD service doesn't provide paging related
> attributes of memory block. Since this issue has been fixed, GCD services
> can be used to determine the paging status of page 0. This is also make it
> possible to just use a new macro
>
> ACCESS_PAGE0_CODE(
> {
> <code accessing page 0>
> }
> );
>
> to replace above methods to do the same job, which also makes code more
> readability.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
> .../Csm/LegacyBiosDxe/LegacyBda.c | 53 ++++----
> .../Csm/LegacyBiosDxe/LegacyBios.c | 135 ++-------------------
> .../Csm/LegacyBiosDxe/LegacyBiosDxe.inf | 1 -
> .../Csm/LegacyBiosDxe/LegacyBiosInterface.h | 16 ---
> .../Csm/LegacyBiosDxe/LegacyBootSupport.c | 80 ++++++------
> .../Csm/LegacyBiosDxe/LegacyPci.c | 72 ++++++-----
> IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c | 51 ++++----
> 7 files changed, 135 insertions(+), 273 deletions(-)
>
> diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
> index c6670febee..9667dc2a0f 100644
> --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
> +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBda.c
> @@ -34,37 +34,36 @@ LegacyBiosInitBda (
> BDA_STRUC *Bda;
> UINT8 *Ebda;
>
> - DisableNullDetection ();
> -
> Bda = (BDA_STRUC *) ((UINTN) 0x400);
> Ebda = (UINT8 *) ((UINTN) 0x9fc00);
>
> - ZeroMem (Bda, 0x100);
> + ACCESS_PAGE0_CODE ({
> + ZeroMem (Bda, 0x100);
> + //
> + // 640k-1k for EBDA
> + //
> + Bda->MemSize = 0x27f;
> + Bda->KeyHead = 0x1e;
> + Bda->KeyTail = 0x1e;
> + Bda->FloppyData = 0x00;
> + Bda->FloppyTimeout = 0xff;
> +
> + Bda->KeyStart = 0x001E;
> + Bda->KeyEnd = 0x003E;
> + Bda->KeyboardStatus = 0x10;
> + Bda->Ebda = 0x9fc0;
> +
> + //
> + // Move LPT time out here and zero out LPT4 since some SCSI OPROMS
> + // use this as scratch pad (LPT4 is Reserved)
> + //
> + Bda->Lpt1_2Timeout = 0x1414;
> + Bda->Lpt3_4Timeout = 0x1400;
> +
> + });
> +
> ZeroMem (Ebda, 0x400);
> - //
> - // 640k-1k for EBDA
> - //
> - Bda->MemSize = 0x27f;
> - Bda->KeyHead = 0x1e;
> - Bda->KeyTail = 0x1e;
> - Bda->FloppyData = 0x00;
> - Bda->FloppyTimeout = 0xff;
> -
> - Bda->KeyStart = 0x001E;
> - Bda->KeyEnd = 0x003E;
> - Bda->KeyboardStatus = 0x10;
> - Bda->Ebda = 0x9fc0;
> -
> - //
> - // Move LPT time out here and zero out LPT4 since some SCSI OPROMS
> - // use this as scratch pad (LPT4 is Reserved)
> - //
> - Bda->Lpt1_2Timeout = 0x1414;
> - Bda->Lpt3_4Timeout = 0x1400;
> -
> - *Ebda = 0x01;
> -
> - EnableNullDetection ();
> + *Ebda = 0x01;
>
> return EFI_SUCCESS;
> }
> diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
> index c6461f5547..d50c15eacb 100644
> --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
> +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c
> @@ -786,115 +786,6 @@ ToggleEndOfDxeStatus (
> return;
> }
>
> -//
> -// Legacy BIOS needs to access memory between 0-4095, which will cause page
> -// fault exception if NULL pointer detection mechanism is enabled. Following
> -// functions can be used to disable/enable NULL pointer detection before/after
> -// accessing those memory.
> -//
> -
> -/**
> - Enable NULL pointer detection.
> -**/
> -VOID
> -EnableNullDetection (
> - VOID
> - )
> -{
> - EFI_STATUS Status;
> - EFI_GCD_MEMORY_SPACE_DESCRIPTOR Desc;
> -
> - if (((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0)
> - ||
> - ((mEndOfDxe) &&
> - ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT7|BIT0))
> - == (BIT7|BIT0)))
> - ) {
> - return;
> - }
> -
> - //
> - // Check current capabilities and attributes
> - //
> - Status = gDS->GetMemorySpaceDescriptor (0, &Desc);
> - ASSERT_EFI_ERROR (Status);
> -
> - //
> - // Try to add EFI_MEMORY_RP support if necessary
> - //
> - if ((Desc.Capabilities & EFI_MEMORY_RP) == 0) {
> - Desc.Capabilities |= EFI_MEMORY_RP;
> - Status = gDS->SetMemorySpaceCapabilities (0, EFI_PAGES_TO_SIZE(1),
> - Desc.Capabilities);
> - ASSERT_EFI_ERROR (Status);
> - if (EFI_ERROR (Status)) {
> - return;
> - }
> - }
> -
> - //
> - // Don't bother if EFI_MEMORY_RP is already set.
> - //
> - if ((Desc.Attributes & EFI_MEMORY_RP) == 0) {
> - Desc.Attributes |= EFI_MEMORY_RP;
> - Status = gDS->SetMemorySpaceAttributes (0, EFI_PAGES_TO_SIZE(1),
> - Desc.Attributes);
> - ASSERT_EFI_ERROR (Status);
> - }
> -}
> -
> -/**
> - Disable NULL pointer detection.
> -**/
> -VOID
> -DisableNullDetection (
> - VOID
> - )
> -{
> - EFI_STATUS Status;
> - EFI_GCD_MEMORY_SPACE_DESCRIPTOR Desc;
> -
> - if (((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0)
> - ||
> - ((mEndOfDxe) &&
> - ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT7|BIT0))
> - == (BIT7|BIT0)))
> - ) {
> - return;
> - }
> -
> - //
> - // Check current capabilities and attributes
> - //
> - Status = gDS->GetMemorySpaceDescriptor (0, &Desc);
> - ASSERT_EFI_ERROR (Status);
> -
> - //
> - // Try to add EFI_MEMORY_RP support if necessary
> - //
> - if ((Desc.Capabilities & EFI_MEMORY_RP) == 0) {
> - Desc.Capabilities |= EFI_MEMORY_RP;
> - Status = gDS->SetMemorySpaceCapabilities (0, EFI_PAGES_TO_SIZE(1),
> - Desc.Capabilities);
> - ASSERT_EFI_ERROR (Status);
> - if (EFI_ERROR (Status)) {
> - return;
> - }
> - }
> -
> - //
> - // Don't bother if EFI_MEMORY_RP is already cleared.
> - //
> - if ((Desc.Attributes & EFI_MEMORY_RP) != 0) {
> - Desc.Attributes &= ~EFI_MEMORY_RP;
> - Status = gDS->SetMemorySpaceAttributes (0, EFI_PAGES_TO_SIZE(1),
> - Desc.Attributes);
> - ASSERT_EFI_ERROR (Status);
> - } else {
> - DEBUG ((DEBUG_WARN, "!!! Page 0 is supposed to be disabled !!!\r\n"));
> - }
> -}
> -
> /**
> Install Driver to produce Legacy BIOS protocol.
>
> @@ -1095,10 +986,10 @@ LegacyBiosInstall (
> // Initialize region from 0x0000 to 4k. This initializes interrupt vector
> // range.
> //
> - DisableNullDetection ();
> - gBS->SetMem ((VOID *) ClearPtr, 0x400, INITIAL_VALUE_BELOW_1K);
> - ZeroMem ((VOID *) ((UINTN)ClearPtr + 0x400), 0xC00);
> - EnableNullDetection ();
> + ACCESS_PAGE0_CODE ({
> + gBS->SetMem ((VOID *) ClearPtr, 0x400, INITIAL_VALUE_BELOW_1K);
> + ZeroMem ((VOID *) ((UINTN)ClearPtr + 0x400), 0xC00);
> + });
>
> //
> // Allocate pages for OPROM usage
> @@ -1237,16 +1128,14 @@ LegacyBiosInstall (
> //
> // Save Unexpected interrupt vector so can restore it just prior to boot
> //
> - DisableNullDetection ();
> -
> - BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
> - Private->BiosUnexpectedInt = BaseVectorMaster[0];
> - IntRedirCode = (UINT32) (UINTN) Private->IntThunk->InterruptRedirectionCode;
> - for (Index = 0; Index < 8; Index++) {
> - BaseVectorMaster[Index] = (EFI_SEGMENT (IntRedirCode + Index * 4) << 16) | EFI_OFFSET (IntRedirCode + Index * 4);
> - }
> -
> - EnableNullDetection ();
> + ACCESS_PAGE0_CODE ({
> + BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
> + Private->BiosUnexpectedInt = BaseVectorMaster[0];
> + IntRedirCode = (UINT32) (UINTN) Private->IntThunk->InterruptRedirectionCode;
> + for (Index = 0; Index < 8; Index++) {
> + BaseVectorMaster[Index] = (EFI_SEGMENT (IntRedirCode + Index * 4) << 16) | EFI_OFFSET (IntRedirCode + Index * 4);
> + }
> + });
>
> //
> // Save EFI value
> diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
> index 6efc7f36ae..180c18e3fc 100644
> --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
> +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosDxe.inf
> @@ -148,7 +148,6 @@
> gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdHighPmmMemorySize ## CONSUMES
> gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdOpromReservedMemoryBase ## CONSUMES
> gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdOpromReservedMemorySize ## CONSUMES
> - gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask ## CONSUMES
>
> [Depex]
> gEfiLegacyRegion2ProtocolGuid AND gEfiLegacyInterruptProtocolGuid AND gEfiLegacyBiosPlatformProtocolGuid AND gEfiLegacy8259ProtocolGuid AND gEfiGenericMemTestProtocolGuid AND gEfiCpuArchProtocolGuid AND gEfiTimerArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid
> diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
> index 86a3b09080..cc2fc9fc13 100644
> --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
> +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBiosInterface.h
> @@ -1544,20 +1544,4 @@ LegacyBiosInstallVgaRom (
> IN LEGACY_BIOS_INSTANCE *Private
> );
>
> -/**
> - Enable NULL pointer detection.
> -**/
> -VOID
> -EnableNullDetection (
> - VOID
> - );
> -
> -/**
> - Disable NULL pointer detection.
> -**/
> -VOID
> -DisableNullDetection (
> - VOID
> - );
> -
> #endif
> diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
> index c2ac69ce69..d65a751fe7 100644
> --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
> +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBootSupport.c
> @@ -1041,7 +1041,9 @@ GenericLegacyBoot (
> //
> // Setup BDA and EBDA standard areas before Legacy Boot
> //
> - LegacyBiosCompleteBdaBeforeBoot (Private);
> + ACCESS_PAGE0_CODE ({
> + LegacyBiosCompleteBdaBeforeBoot (Private);
> + });
> LegacyBiosCompleteStandardCmosBeforeBoot (Private);
>
> //
> @@ -1073,10 +1075,10 @@ GenericLegacyBoot (
> // Use 182/10 to avoid floating point math.
> //
> LocalTime = (LocalTime * 182) / 10;
> - DisableNullDetection ();
> - BdaPtr = (UINT32 *) (UINTN)0x46C;
> - *BdaPtr = LocalTime;
> - EnableNullDetection ();
> + ACCESS_PAGE0_CODE ({
> + BdaPtr = (UINT32 *) (UINTN)0x46C;
> + *BdaPtr = LocalTime;
> + });
>
> //
> // Shadow PCI ROMs. We must do this near the end since this will kick
> @@ -1322,15 +1324,15 @@ GenericLegacyBoot (
> // set of TIANO vectors) or takes it over.
> //
> //
> - DisableNullDetection ();
> - BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
> - for (Index = 0; Index < 8; Index++) {
> - Private->ThunkSavedInt[Index] = BaseVectorMaster[Index];
> - if (Private->ThunkSeg == (UINT16) (BaseVectorMaster[Index] >> 16)) {
> - BaseVectorMaster[Index] = (UINT32) (Private->BiosUnexpectedInt);
> + ACCESS_PAGE0_CODE ({
> + BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
> + for (Index = 0; Index < 8; Index++) {
> + Private->ThunkSavedInt[Index] = BaseVectorMaster[Index];
> + if (Private->ThunkSeg == (UINT16) (BaseVectorMaster[Index] >> 16)) {
> + BaseVectorMaster[Index] = (UINT32) (Private->BiosUnexpectedInt);
> + }
> }
> - }
> - EnableNullDetection ();
> + });
>
> ZeroMem (&Regs, sizeof (EFI_IA32_REGISTER_SET));
> Regs.X.AX = Legacy16Boot;
> @@ -1344,12 +1346,12 @@ GenericLegacyBoot (
> 0
> );
>
> - DisableNullDetection ();
> - BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
> - for (Index = 0; Index < 8; Index++) {
> - BaseVectorMaster[Index] = Private->ThunkSavedInt[Index];
> - }
> - EnableNullDetection ();
> + ACCESS_PAGE0_CODE ({
> + BaseVectorMaster = (UINT32 *) (sizeof (UINT32) * PROTECTED_MODE_BASE_VECTOR_MASTER);
> + for (Index = 0; Index < 8; Index++) {
> + BaseVectorMaster[Index] = Private->ThunkSavedInt[Index];
> + }
> + });
> }
> Private->LegacyBootEntered = TRUE;
> if ((mBootMode == BOOT_LEGACY_OS) || (mBootMode == BOOT_UNCONVENTIONAL_DEVICE)) {
> @@ -1737,11 +1739,11 @@ LegacyBiosBuildE820 (
> //
> // First entry is 0 to (640k - EBDA)
> //
> - DisableNullDetection ();
> - E820Table[0].BaseAddr = 0;
> - E820Table[0].Length = (UINT64) ((*(UINT16 *) (UINTN)0x40E) << 4);
> - E820Table[0].Type = EfiAcpiAddressRangeMemory;
> - EnableNullDetection ();
> + ACCESS_PAGE0_CODE ({
> + E820Table[0].BaseAddr = 0;
> + E820Table[0].Length = (UINT64) ((*(UINT16 *) (UINTN)0x40E) << 4);
> + E820Table[0].Type = EfiAcpiAddressRangeMemory;
> + });
>
> //
> // Second entry is (640k - EBDA) to 640k
> @@ -1975,8 +1977,6 @@ LegacyBiosCompleteBdaBeforeBoot (
> UINT16 MachineConfig;
> DEVICE_PRODUCER_DATA_HEADER *SioPtr;
>
> - DisableNullDetection ();
> -
> Bda = (BDA_STRUC *) ((UINTN) 0x400);
> MachineConfig = 0;
>
> @@ -2035,8 +2035,6 @@ LegacyBiosCompleteBdaBeforeBoot (
> MachineConfig = (UINT16) (MachineConfig + 0x00 + 0x02 + (SioPtr->MousePresent * 0x04));
> Bda->MachineConfig = MachineConfig;
>
> - EnableNullDetection ();
> -
> return EFI_SUCCESS;
> }
>
> @@ -2063,17 +2061,15 @@ LegacyBiosUpdateKeyboardLedStatus (
>
> Private = LEGACY_BIOS_INSTANCE_FROM_THIS (This);
>
> - DisableNullDetection ();
> -
> - Bda = (BDA_STRUC *) ((UINTN) 0x400);
> - LocalLeds = Leds;
> - Bda->LedStatus = (UINT8) ((Bda->LedStatus &~0x07) | LocalLeds);
> - LocalLeds = (UINT8) (LocalLeds << 4);
> - Bda->ShiftStatus = (UINT8) ((Bda->ShiftStatus &~0x70) | LocalLeds);
> - LocalLeds = (UINT8) (Leds & 0x20);
> - Bda->KeyboardStatus = (UINT8) ((Bda->KeyboardStatus &~0x20) | LocalLeds);
> -
> - EnableNullDetection ();
> + ACCESS_PAGE0_CODE ({
> + Bda = (BDA_STRUC *) ((UINTN) 0x400);
> + LocalLeds = Leds;
> + Bda->LedStatus = (UINT8) ((Bda->LedStatus &~0x07) | LocalLeds);
> + LocalLeds = (UINT8) (LocalLeds << 4);
> + Bda->ShiftStatus = (UINT8) ((Bda->ShiftStatus &~0x70) | LocalLeds);
> + LocalLeds = (UINT8) (Leds & 0x20);
> + Bda->KeyboardStatus = (UINT8) ((Bda->KeyboardStatus &~0x20) | LocalLeds);
> + });
>
> //
> // Call into Legacy16 code to allow it to do any processing
> @@ -2119,9 +2115,9 @@ LegacyBiosCompleteStandardCmosBeforeBoot (
> // to large capacity drives
> // CMOS 14 = BDA 40:10 plus bit 3(display enabled)
> //
> - DisableNullDetection ();
> - Bda = (UINT8)(*((UINT8 *)((UINTN)0x410)) | BIT3);
> - EnableNullDetection ();
> + ACCESS_PAGE0_CODE ({
> + Bda = (UINT8)(*((UINT8 *)((UINTN)0x410)) | BIT3);
> + });
>
> //
> // Force display enabled
> diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
> index d38cef3e33..c317d21055 100644
> --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
> +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyPci.c
> @@ -2403,35 +2403,33 @@ LegacyBiosInstallRom (
> // 2. BBS compliants drives will not change 40:75 until boot time.
> // 3. Onboard IDE controllers will change 40:75
> //
> - DisableNullDetection ();
> -
> - LocalDiskStart = (UINT8) ((*(UINT8 *) ((UINTN) 0x475)) + 0x80);
> - if ((Private->Disk4075 + 0x80) < LocalDiskStart) {
> - //
> - // Update table since onboard IDE drives found
> - //
> - Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].PciSegment = 0xff;
> - Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].PciBus = 0xff;
> - Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].PciDevice = 0xff;
> - Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].PciFunction = 0xff;
> - Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].StartDriveNumber = (UINT8) (Private->Disk4075 + 0x80);
> - Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].EndDriveNumber = LocalDiskStart;
> - Private->LegacyEfiHddTableIndex ++;
> - Private->Disk4075 = (UINT8) (LocalDiskStart & 0x7f);
> - Private->DiskEnd = LocalDiskStart;
> - }
> -
> - if (PciHandle != mVgaHandle) {
> + ACCESS_PAGE0_CODE ({
> + LocalDiskStart = (UINT8) ((*(UINT8 *) ((UINTN) 0x475)) + 0x80);
> + if ((Private->Disk4075 + 0x80) < LocalDiskStart) {
> + //
> + // Update table since onboard IDE drives found
> + //
> + Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].PciSegment = 0xff;
> + Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].PciBus = 0xff;
> + Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].PciDevice = 0xff;
> + Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].PciFunction = 0xff;
> + Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].StartDriveNumber = (UINT8) (Private->Disk4075 + 0x80);
> + Private->LegacyEfiHddTable[Private->LegacyEfiHddTableIndex].EndDriveNumber = LocalDiskStart;
> + Private->LegacyEfiHddTableIndex ++;
> + Private->Disk4075 = (UINT8) (LocalDiskStart & 0x7f);
> + Private->DiskEnd = LocalDiskStart;
> + }
>
> - EnablePs2Keyboard ();
> + if (PciHandle != mVgaHandle) {
>
> - //
> - // Store current mode settings since PrepareToScanRom may change mode.
> - //
> - VideoMode = *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE));
> - }
> + EnablePs2Keyboard ();
>
> - EnableNullDetection ();
> + //
> + // Store current mode settings since PrepareToScanRom may change mode.
> + //
> + VideoMode = *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE));
> + }
> + });
>
> //
> // Notify the platform that we are about to scan the ROM
> @@ -2473,11 +2471,11 @@ LegacyBiosInstallRom (
> // Multiply result by 18.2 for number of ticks since midnight.
> // Use 182/10 to avoid floating point math.
> //
> - DisableNullDetection ();
> - LocalTime = (LocalTime * 182) / 10;
> - BdaPtr = (UINT32 *) ((UINTN) 0x46C);
> - *BdaPtr = LocalTime;
> - EnableNullDetection ();
> + ACCESS_PAGE0_CODE ({
> + LocalTime = (LocalTime * 182) / 10;
> + BdaPtr = (UINT32 *) ((UINTN) 0x46C);
> + *BdaPtr = LocalTime;
> + });
>
> //
> // Pass in handoff data
> @@ -2573,9 +2571,9 @@ LegacyBiosInstallRom (
> //
> // Set mode settings since PrepareToScanRom may change mode
> //
> - DisableNullDetection ();
> - OldVideoMode = *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE));
> - EnableNullDetection ();
> + ACCESS_PAGE0_CODE ({
> + OldVideoMode = *(UINT8 *) ((UINTN) (0x400 + BDA_VIDEO_MODE));
> + });
>
> if (VideoMode != OldVideoMode) {
> //
> @@ -2617,9 +2615,9 @@ LegacyBiosInstallRom (
> }
> }
>
> - DisableNullDetection ();
> - LocalDiskEnd = (UINT8) ((*(UINT8 *) ((UINTN) 0x475)) + 0x80);
> - EnableNullDetection ();
> + ACCESS_PAGE0_CODE ({
> + LocalDiskEnd = (UINT8) ((*(UINT8 *) ((UINTN) 0x475)) + 0x80);
> + });
>
> //
> // Allow platform to perform any required actions after the
> diff --git a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
> index d249479c56..d975d94e70 100644
> --- a/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
> +++ b/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/Thunk.c
> @@ -73,10 +73,10 @@ LegacyBiosInt86 (
> // The base address of legacy interrupt vector table is 0.
> // We use this base address to get the legacy interrupt handler.
> //
> - DisableNullDetection ();
> - Segment = (UINT16)(((UINT32 *)0)[BiosInt] >> 16);
> - Offset = (UINT16)((UINT32 *)0)[BiosInt];
> - EnableNullDetection ();
> + ACCESS_PAGE0_CODE ({
{} is not needed, right?
> + Segment = (UINT16)(((UINT32 *)0)[BiosInt] >> 16);
> + Offset = (UINT16)((UINT32 *)0)[BiosInt];
> + });
>
> return InternalLegacyBiosFarCall (
> This,
> @@ -286,29 +286,6 @@ InternalLegacyBiosFarCall (
>
> AsmThunk16 (&mThunkContext);
>
> - //
> - // OPROM may allocate EBDA range by itself and change EBDA base and EBDA size.
> - // Get the current EBDA base address, and compared with pre-allocate minimum
> - // EBDA base address, if the current EBDA base address is smaller, it indicates
> - // PcdEbdaReservedMemorySize should be adjusted to larger for more OPROMs.
> - //
> - DEBUG_CODE (
> - {
> - UINTN EbdaBaseAddress;
> - UINTN ReservedEbdaBaseAddress;
> -
> - //
> - // Skip this part of debug code if NULL pointer detection is enabled
> - //
> - if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) == 0) {
> - EbdaBaseAddress = (*(UINT16 *) (UINTN) 0x40E) << 4;
> - ReservedEbdaBaseAddress = CONVENTIONAL_MEMORY_TOP
> - - PcdGet32 (PcdEbdaReservedMemorySize);
> - ASSERT (ReservedEbdaBaseAddress <= EbdaBaseAddress);
> - }
> - }
> - );
> -
> if (Stack != NULL && StackSize != 0) {
> //
> // Copy low memory stack to Stack
> @@ -334,6 +311,26 @@ InternalLegacyBiosFarCall (
> //
> gBS->RestoreTPL (OriginalTpl);
>
> + //
> + // OPROM may allocate EBDA range by itself and change EBDA base and EBDA size.
> + // Get the current EBDA base address, and compared with pre-allocate minimum
> + // EBDA base address, if the current EBDA base address is smaller, it indicates
> + // PcdEbdaReservedMemorySize should be adjusted to larger for more OPROMs.
> + //
> + DEBUG_CODE (
> + {
> + UINTN EbdaBaseAddress;
> + UINTN ReservedEbdaBaseAddress;
> +
> + ACCESS_PAGE0_CODE ({
> + EbdaBaseAddress = (*(UINT16 *) (UINTN) 0x40E) << 4;
> + ReservedEbdaBaseAddress = CONVENTIONAL_MEMORY_TOP
> + - PcdGet32 (PcdEbdaReservedMemorySize);
> + ASSERT (ReservedEbdaBaseAddress <= EbdaBaseAddress);
> + });
> + }
> + );
> +
> //
> // Restore interrupt of debug timer
> //
>
--
Thanks,
Ray
next prev parent reply other threads:[~2017-12-06 9:27 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-06 7:31 [PATCH 0/3] Remove dependency on PcdNullPointerDetectionPropertyMask Jian J Wang
2017-12-06 7:31 ` [PATCH 1/3] IntelFrameworkPkg/LegacyBios.h: Add a macro to guarantee page 0 access Jian J Wang
2017-12-06 9:31 ` Ni, Ruiyu
2017-12-06 12:36 ` Wang, Jian J
2017-12-06 7:31 ` [PATCH 2/3] IntelFrameworkModulePkg/LegacyBiosDxe: Use macro to enable/disable page 0 Jian J Wang
2017-12-06 9:32 ` Ni, Ruiyu [this message]
2017-12-06 12:38 ` Wang, Jian J
2017-12-06 7:31 ` [PATCH 3/3] IntelFrameworkModulePkg/KeyboardDxe: " Jian J Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8504df95-2c80-bc7b-c928-084021649a90@Intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox