From: "Wang, Jian J" <jian.j.wang@intel.com>
To: "Ni, Ruiyu" <ruiyu.ni@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>,
"Gao, Liming" <liming.gao@intel.com>
Subject: Re: [PATCH 2/3] IntelFrameworkModulePkg/LegacyBiosDxe: Use macro to enable/disable page 0
Date: Wed, 6 Dec 2017 12:38:38 +0000 [thread overview]
Message-ID: <D827630B58408649ACB04F44C510003624CBB93B@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <8504df95-2c80-bc7b-c928-084021649a90@Intel.com>
You're right that {} are not necessary. I'm wondering why I can't pass build without them before.
They'll be removed in v2.
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Wednesday, December 06, 2017 5:32 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH 2/3] IntelFrameworkModulePkg/LegacyBiosDxe: Use
> macro to enable/disable page 0
>
> 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.PcdOpromReservedMemoryBas
> e ## 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 12:34 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
2017-12-06 12:38 ` Wang, Jian J [this message]
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=D827630B58408649ACB04F44C510003624CBB93B@SHSMSX103.ccr.corp.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