* [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib
2018-10-12 14:40 [PATCH 0/6] Add kvmtool emulated platform support for ARM Sami Mujawar
@ 2018-10-12 14:40 ` Sami Mujawar
2018-10-12 14:49 ` Ard Biesheuvel
2018-10-12 14:40 ` [PATCH v1 2/6] PcAtChipsetPkg: Add MMIO Support to RTC driver Sami Mujawar
` (5 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Sami Mujawar @ 2018-10-12 14:40 UTC (permalink / raw)
To: edk2-devel
Cc: ard.biesheuvel, leif.lindholm, ruiyu.ni, evan.lloyd,
Matteo.Carlini, Stephanie.Hughes-Fitt, nd
Some virtual machine managers like kvmtool emulate the PC AT
Serial Port UART in the MMIO space so that architectures that
do not support I/O Mapped I/O can use the UART.
This patch adds MMIO support to the PC AT SerialPortLib.
PcdSerialUseMmio is used to select I/O or MMIO support.
If PcdSerialUseMmio is
TRUE - The value is read/written from MMIO space.
FALSE - The value is read/written from I/O space.
The Default is I/O space.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
The changes can be seen at https://github.com/samimujawar/edk2/commit/84f908387d2031def5d374759e7ad4cf90786c91
Notes:
v1:
- Add support to read/write from UART registers using MMIO access [SAMI]
PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf | 4 +
PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c | 98 ++++++++++++++++----
2 files changed, 84 insertions(+), 18 deletions(-)
diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
index 959d6e27c9812d201d44d9249070ab7758cbfe00..d0689793040fd930701b02dae51ff59ea16a10c4 100644
--- a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
+++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
@@ -2,6 +2,7 @@
# Library instance for SerialIo library class
#
# Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
# which accompanies this distribution. The full text of the license may be found at
@@ -23,6 +24,7 @@ [Defines]
[Packages]
MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
[LibraryClasses]
BaseLib
@@ -31,3 +33,5 @@ [LibraryClasses]
[Sources]
SerialPortLib.c
+[FixedPcd]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio ## CONSUMES
diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
index d1a1c6a03facad09e781b5605e22a24e5f51c618..898e5f957aae998624189c6b538912da0439dfe8 100644
--- a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
+++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
@@ -2,6 +2,8 @@
UART Serial Port library functions
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
+
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -57,6 +59,66 @@ UINT8 gParity = 0;
UINT8 gBreakSet = 0;
/**
+ Reads an 8-bit value from the serial port.
+
+ This function checks PcdSerialUseMmio to determine if I/O
+ mapped IO or Memory Mapped IO operations must be performed on
+ the serial port. It then uses the appropriate interface to
+ read the Value from the serial port.
+
+ If PcdSerialUseMmio is TRUE, then the value is read from MMIO space.
+ If PcdSerialUseMmio is FALSE, then the value is read from I/O space.
+
+ @param Address The UART register to read from.
+
+ @return The value read from the serial port.
+
+**/
+STATIC
+UINT8
+SerialPortRead8 (
+ IN UINTN Address
+)
+{
+ if (FixedPcdGetBool (PcdSerialUseMmio)) {
+ return MmioRead8 (Address);
+ }
+
+ return IoRead8 (Address);
+}
+
+/**
+ Writes an 8-bit value to the serial port.
+
+ This function checks PcdSerialUseMmio to determine if I/O
+ mapped IO or Memory Mapped IO operations must be performed on
+ the serial port. It then uses the appropriate interface to
+ write the Value to the serial port.
+
+ If PcdSerialUseMmio is TRUE, then the value is written to MMIO space.
+ If PcdSerialUseMmio is FALSE, then the value is written to I/O space.
+
+ @param Address The UART register to write.
+ @param Value The value to write to the I/O port.
+
+ @return The value written to the serial port.
+
+**/
+STATIC
+UINT8
+SerialPortWrite8 (
+ IN UINTN Address,
+ IN UINT8 Value
+)
+{
+ if (FixedPcdGetBool (PcdSerialUseMmio)) {
+ return MmioWrite8 (Address, Value);
+ }
+
+ return IoWrite8 (Address, Value);
+}
+
+/**
Initialize the serial device hardware.
If no initialization is required, then return RETURN_SUCCESS.
@@ -91,19 +153,19 @@ SerialPortInitialize (
// Set communications format
//
OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (gParity << 3) | (gStop << 2) | Data);
- IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
+ SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
//
// Configure baud rate
//
- IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
- IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
+ SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
+ SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
//
// Switch back to bank 0
//
OutputData = (UINT8) ( (gBreakSet << 6) | (gParity << 3) | (gStop << 2) | Data);
- IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
+ SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
return RETURN_SUCCESS;
}
@@ -148,9 +210,9 @@ SerialPortWrite (
// Wait for the serail port to be ready.
//
do {
- Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
+ Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
} while ((Data & LSR_TXRDY) == 0);
- IoWrite8 ((UINT16) gUartBase, *Buffer++);
+ SerialPortWrite8 ((UINT16) gUartBase, *Buffer++);
}
return Result;
@@ -189,10 +251,10 @@ SerialPortRead (
// Wait for the serail port to be ready.
//
do {
- Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
+ Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
} while ((Data & LSR_RXDA) == 0);
- *Buffer++ = IoRead8 ((UINT16) gUartBase);
+ *Buffer++ = SerialPortRead8 ((UINT16) gUartBase);
}
return Result;
@@ -220,7 +282,7 @@ SerialPortPoll (
//
// Read the serial port status.
//
- Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
+ Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
return (BOOLEAN) ((Data & LSR_RXDA) != 0);
}
@@ -253,7 +315,7 @@ SerialPortSetControl (
//
// Read the Modem Control Register.
//
- Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET);
+ Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET);
Mcr &= (~(MCR_DTRC | MCR_RTS));
if ((Control & EFI_SERIAL_DATA_TERMINAL_READY) == EFI_SERIAL_DATA_TERMINAL_READY) {
@@ -267,7 +329,7 @@ SerialPortSetControl (
//
// Write the Modem Control Register.
//
- IoWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr);
+ SerialPortWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr);
return RETURN_SUCCESS;
}
@@ -297,7 +359,7 @@ SerialPortGetControl (
//
// Read the Modem Status Register.
//
- Msr = IoRead8 ((UINT16) gUartBase + MSR_OFFSET);
+ Msr = SerialPortRead8 ((UINT16) gUartBase + MSR_OFFSET);
if ((Msr & MSR_CTS) == MSR_CTS) {
*Control |= EFI_SERIAL_CLEAR_TO_SEND;
@@ -318,7 +380,7 @@ SerialPortGetControl (
//
// Read the Modem Control Register.
//
- Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET);
+ Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET);
if ((Mcr & MCR_DTRC) == MCR_DTRC) {
*Control |= EFI_SERIAL_DATA_TERMINAL_READY;
@@ -331,7 +393,7 @@ SerialPortGetControl (
//
// Read the Line Status Register.
//
- Lsr = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
+ Lsr = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
if ((Lsr & LSR_TXRDY) == LSR_TXRDY) {
*Control |= EFI_SERIAL_OUTPUT_BUFFER_EMPTY;
@@ -470,19 +532,19 @@ SerialPortSetAttributes (
// Set communications format
//
OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (LcrParity << 3) | (LcrStop << 2) | LcrData);
- IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
+ SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
//
// Configure baud rate
//
- IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
- IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
+ SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
+ SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
//
// Switch back to bank 0
//
OutputData = (UINT8) ((gBreakSet << 6) | (LcrParity << 3) | (LcrStop << 2) | LcrData);
- IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
+ SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
return RETURN_SUCCESS;
}
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib
2018-10-12 14:40 ` [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib Sami Mujawar
@ 2018-10-12 14:49 ` Ard Biesheuvel
2018-10-12 15:06 ` Sami Mujawar
0 siblings, 1 reply; 21+ messages in thread
From: Ard Biesheuvel @ 2018-10-12 14:49 UTC (permalink / raw)
To: Sami Mujawar
Cc: edk2-devel@lists.01.org, Leif Lindholm, Ruiyu Ni, Evan Lloyd,
Matteo Carlini, Stephanie Hughes-Fitt, nd
On 12 October 2018 at 16:40, Sami Mujawar <sami.mujawar@arm.com> wrote:
> Some virtual machine managers like kvmtool emulate the PC AT
> Serial Port UART in the MMIO space so that architectures that
> do not support I/O Mapped I/O can use the UART.
>
> This patch adds MMIO support to the PC AT SerialPortLib.
>
> PcdSerialUseMmio is used to select I/O or MMIO support.
> If PcdSerialUseMmio is
> TRUE - The value is read/written from MMIO space.
> FALSE - The value is read/written from I/O space.
> The Default is I/O space.
>
IIUC kvmtool exposes a 8250, and our 8250 driver already implements
support for MMIO mapped registers. Can't you use that instead?
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> The changes can be seen at https://github.com/samimujawar/edk2/commit/84f908387d2031def5d374759e7ad4cf90786c91
>
> Notes:
> v1:
> - Add support to read/write from UART registers using MMIO access [SAMI]
>
> PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf | 4 +
> PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c | 98 ++++++++++++++++----
> 2 files changed, 84 insertions(+), 18 deletions(-)
>
> diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> index 959d6e27c9812d201d44d9249070ab7758cbfe00..d0689793040fd930701b02dae51ff59ea16a10c4 100644
> --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> @@ -2,6 +2,7 @@
> # Library instance for SerialIo library class
> #
> # Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
> # which accompanies this distribution. The full text of the license may be found at
> @@ -23,6 +24,7 @@ [Defines]
>
> [Packages]
> MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
>
> [LibraryClasses]
> BaseLib
> @@ -31,3 +33,5 @@ [LibraryClasses]
> [Sources]
> SerialPortLib.c
>
> +[FixedPcd]
> + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio ## CONSUMES
> diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> index d1a1c6a03facad09e781b5605e22a24e5f51c618..898e5f957aae998624189c6b538912da0439dfe8 100644
> --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> @@ -2,6 +2,8 @@
> UART Serial Port library functions
>
> Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
> +
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> @@ -57,6 +59,66 @@ UINT8 gParity = 0;
> UINT8 gBreakSet = 0;
>
> /**
> + Reads an 8-bit value from the serial port.
> +
> + This function checks PcdSerialUseMmio to determine if I/O
> + mapped IO or Memory Mapped IO operations must be performed on
> + the serial port. It then uses the appropriate interface to
> + read the Value from the serial port.
> +
> + If PcdSerialUseMmio is TRUE, then the value is read from MMIO space.
> + If PcdSerialUseMmio is FALSE, then the value is read from I/O space.
> +
> + @param Address The UART register to read from.
> +
> + @return The value read from the serial port.
> +
> +**/
> +STATIC
> +UINT8
> +SerialPortRead8 (
> + IN UINTN Address
> +)
> +{
> + if (FixedPcdGetBool (PcdSerialUseMmio)) {
> + return MmioRead8 (Address);
> + }
> +
> + return IoRead8 (Address);
> +}
> +
> +/**
> + Writes an 8-bit value to the serial port.
> +
> + This function checks PcdSerialUseMmio to determine if I/O
> + mapped IO or Memory Mapped IO operations must be performed on
> + the serial port. It then uses the appropriate interface to
> + write the Value to the serial port.
> +
> + If PcdSerialUseMmio is TRUE, then the value is written to MMIO space.
> + If PcdSerialUseMmio is FALSE, then the value is written to I/O space.
> +
> + @param Address The UART register to write.
> + @param Value The value to write to the I/O port.
> +
> + @return The value written to the serial port.
> +
> +**/
> +STATIC
> +UINT8
> +SerialPortWrite8 (
> + IN UINTN Address,
> + IN UINT8 Value
> +)
> +{
> + if (FixedPcdGetBool (PcdSerialUseMmio)) {
> + return MmioWrite8 (Address, Value);
> + }
> +
> + return IoWrite8 (Address, Value);
> +}
> +
> +/**
> Initialize the serial device hardware.
>
> If no initialization is required, then return RETURN_SUCCESS.
> @@ -91,19 +153,19 @@ SerialPortInitialize (
> // Set communications format
> //
> OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (gParity << 3) | (gStop << 2) | Data);
> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>
> //
> // Configure baud rate
> //
> - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
> - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
> + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
> + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
>
> //
> // Switch back to bank 0
> //
> OutputData = (UINT8) ( (gBreakSet << 6) | (gParity << 3) | (gStop << 2) | Data);
> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>
> return RETURN_SUCCESS;
> }
> @@ -148,9 +210,9 @@ SerialPortWrite (
> // Wait for the serail port to be ready.
> //
> do {
> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
> } while ((Data & LSR_TXRDY) == 0);
> - IoWrite8 ((UINT16) gUartBase, *Buffer++);
> + SerialPortWrite8 ((UINT16) gUartBase, *Buffer++);
> }
>
> return Result;
> @@ -189,10 +251,10 @@ SerialPortRead (
> // Wait for the serail port to be ready.
> //
> do {
> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
> } while ((Data & LSR_RXDA) == 0);
>
> - *Buffer++ = IoRead8 ((UINT16) gUartBase);
> + *Buffer++ = SerialPortRead8 ((UINT16) gUartBase);
> }
>
> return Result;
> @@ -220,7 +282,7 @@ SerialPortPoll (
> //
> // Read the serial port status.
> //
> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
>
> return (BOOLEAN) ((Data & LSR_RXDA) != 0);
> }
> @@ -253,7 +315,7 @@ SerialPortSetControl (
> //
> // Read the Modem Control Register.
> //
> - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET);
> + Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET);
> Mcr &= (~(MCR_DTRC | MCR_RTS));
>
> if ((Control & EFI_SERIAL_DATA_TERMINAL_READY) == EFI_SERIAL_DATA_TERMINAL_READY) {
> @@ -267,7 +329,7 @@ SerialPortSetControl (
> //
> // Write the Modem Control Register.
> //
> - IoWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr);
> + SerialPortWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr);
>
> return RETURN_SUCCESS;
> }
> @@ -297,7 +359,7 @@ SerialPortGetControl (
> //
> // Read the Modem Status Register.
> //
> - Msr = IoRead8 ((UINT16) gUartBase + MSR_OFFSET);
> + Msr = SerialPortRead8 ((UINT16) gUartBase + MSR_OFFSET);
>
> if ((Msr & MSR_CTS) == MSR_CTS) {
> *Control |= EFI_SERIAL_CLEAR_TO_SEND;
> @@ -318,7 +380,7 @@ SerialPortGetControl (
> //
> // Read the Modem Control Register.
> //
> - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET);
> + Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET);
>
> if ((Mcr & MCR_DTRC) == MCR_DTRC) {
> *Control |= EFI_SERIAL_DATA_TERMINAL_READY;
> @@ -331,7 +393,7 @@ SerialPortGetControl (
> //
> // Read the Line Status Register.
> //
> - Lsr = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
> + Lsr = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
>
> if ((Lsr & LSR_TXRDY) == LSR_TXRDY) {
> *Control |= EFI_SERIAL_OUTPUT_BUFFER_EMPTY;
> @@ -470,19 +532,19 @@ SerialPortSetAttributes (
> // Set communications format
> //
> OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (LcrParity << 3) | (LcrStop << 2) | LcrData);
> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>
> //
> // Configure baud rate
> //
> - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
> - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
> + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
> + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
>
> //
> // Switch back to bank 0
> //
> OutputData = (UINT8) ((gBreakSet << 6) | (LcrParity << 3) | (LcrStop << 2) | LcrData);
> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>
> return RETURN_SUCCESS;
> }
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib
2018-10-12 14:49 ` Ard Biesheuvel
@ 2018-10-12 15:06 ` Sami Mujawar
2018-10-12 15:31 ` Kinney, Michael D
2018-10-12 15:33 ` Ard Biesheuvel
0 siblings, 2 replies; 21+ messages in thread
From: Sami Mujawar @ 2018-10-12 15:06 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Leif Lindholm, Ruiyu Ni, Evan Lloyd,
Matteo Carlini, Stephanie Hughes-Fitt, nd
Hi Ard,
I believe you are referring to MdeModulePkg\Library\BaseSerialPortLib16550\BaseSerialPortLib16550.inf
I have tried using this Serial Port Library. However, this has a dependency on PciLib which is difficult to resolve as we use the Serial port early in the boot.
Please do let me know if you know a workaround to solve this issue.
Regards,
Sami Mujawar
-----Original Message-----
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Sent: 12 October 2018 03:50 PM
To: Sami Mujawar <Sami.Mujawar@arm.com>
Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Ruiyu Ni <ruiyu.ni@intel.com>; Evan Lloyd <Evan.Lloyd@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd <nd@arm.com>
Subject: Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib
On 12 October 2018 at 16:40, Sami Mujawar <sami.mujawar@arm.com> wrote:
> Some virtual machine managers like kvmtool emulate the PC AT Serial
> Port UART in the MMIO space so that architectures that do not support
> I/O Mapped I/O can use the UART.
>
> This patch adds MMIO support to the PC AT SerialPortLib.
>
> PcdSerialUseMmio is used to select I/O or MMIO support.
> If PcdSerialUseMmio is
> TRUE - The value is read/written from MMIO space.
> FALSE - The value is read/written from I/O space.
> The Default is I/O space.
>
IIUC kvmtool exposes a 8250, and our 8250 driver already implements support for MMIO mapped registers. Can't you use that instead?
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> The changes can be seen at
> https://github.com/samimujawar/edk2/commit/84f908387d2031def5d374759e7
> ad4cf90786c91
>
> Notes:
> v1:
> - Add support to read/write from UART registers using MMIO access
> [SAMI]
>
> PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf | 4 +
> PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c | 98
> ++++++++++++++++----
> 2 files changed, 84 insertions(+), 18 deletions(-)
>
> diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> index
> 959d6e27c9812d201d44d9249070ab7758cbfe00..d0689793040fd930701b02dae51f
> f59ea16a10c4 100644
> --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> @@ -2,6 +2,7 @@
> # Library instance for SerialIo library class
> #
> # Copyright (c) 2006 - 2014, Intel Corporation. All rights
> reserved.<BR>
> +# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
> # This program and the accompanying materials # are licensed and
> made available under the terms and conditions of the BSD License #
> which accompanies this distribution. The full text of the license may
> be found at @@ -23,6 +24,7 @@ [Defines]
>
> [Packages]
> MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
>
> [LibraryClasses]
> BaseLib
> @@ -31,3 +33,5 @@ [LibraryClasses]
> [Sources]
> SerialPortLib.c
>
> +[FixedPcd]
> + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio ## CONSUMES
> diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> index
> d1a1c6a03facad09e781b5605e22a24e5f51c618..898e5f957aae998624189c6b5389
> 12da0439dfe8 100644
> --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> @@ -2,6 +2,8 @@
> UART Serial Port library functions
>
> Copyright (c) 2006 - 2018, Intel Corporation. All rights
> reserved.<BR>
> + Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
> +
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> @@ -57,6 +59,66 @@ UINT8 gParity = 0;
> UINT8 gBreakSet = 0;
>
> /**
> + Reads an 8-bit value from the serial port.
> +
> + This function checks PcdSerialUseMmio to determine if I/O mapped
> + IO or Memory Mapped IO operations must be performed on the serial
> + port. It then uses the appropriate interface to read the Value from
> + the serial port.
> +
> + If PcdSerialUseMmio is TRUE, then the value is read from MMIO space.
> + If PcdSerialUseMmio is FALSE, then the value is read from I/O space.
> +
> + @param Address The UART register to read from.
> +
> + @return The value read from the serial port.
> +
> +**/
> +STATIC
> +UINT8
> +SerialPortRead8 (
> + IN UINTN Address
> +)
> +{
> + if (FixedPcdGetBool (PcdSerialUseMmio)) {
> + return MmioRead8 (Address);
> + }
> +
> + return IoRead8 (Address);
> +}
> +
> +/**
> + Writes an 8-bit value to the serial port.
> +
> + This function checks PcdSerialUseMmio to determine if I/O mapped
> + IO or Memory Mapped IO operations must be performed on the serial
> + port. It then uses the appropriate interface to write the Value to
> + the serial port.
> +
> + If PcdSerialUseMmio is TRUE, then the value is written to MMIO space.
> + If PcdSerialUseMmio is FALSE, then the value is written to I/O space.
> +
> + @param Address The UART register to write.
> + @param Value The value to write to the I/O port.
> +
> + @return The value written to the serial port.
> +
> +**/
> +STATIC
> +UINT8
> +SerialPortWrite8 (
> + IN UINTN Address,
> + IN UINT8 Value
> +)
> +{
> + if (FixedPcdGetBool (PcdSerialUseMmio)) {
> + return MmioWrite8 (Address, Value);
> + }
> +
> + return IoWrite8 (Address, Value);
> +}
> +
> +/**
> Initialize the serial device hardware.
>
> If no initialization is required, then return RETURN_SUCCESS.
> @@ -91,19 +153,19 @@ SerialPortInitialize (
> // Set communications format
> //
> OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (gParity <<
> 3) | (gStop << 2) | Data);
> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>
> //
> // Configure baud rate
> //
> - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
> - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
> + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >>
> + 8));
> + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor &
> + 0xff));
>
> //
> // Switch back to bank 0
> //
> OutputData = (UINT8) ( (gBreakSet << 6) | (gParity << 3) | (gStop
> << 2) | Data);
> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>
> return RETURN_SUCCESS;
> }
> @@ -148,9 +210,9 @@ SerialPortWrite (
> // Wait for the serail port to be ready.
> //
> do {
> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
> } while ((Data & LSR_TXRDY) == 0);
> - IoWrite8 ((UINT16) gUartBase, *Buffer++);
> + SerialPortWrite8 ((UINT16) gUartBase, *Buffer++);
> }
>
> return Result;
> @@ -189,10 +251,10 @@ SerialPortRead (
> // Wait for the serail port to be ready.
> //
> do {
> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
> } while ((Data & LSR_RXDA) == 0);
>
> - *Buffer++ = IoRead8 ((UINT16) gUartBase);
> + *Buffer++ = SerialPortRead8 ((UINT16) gUartBase);
> }
>
> return Result;
> @@ -220,7 +282,7 @@ SerialPortPoll (
> //
> // Read the serial port status.
> //
> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
>
> return (BOOLEAN) ((Data & LSR_RXDA) != 0); } @@ -253,7 +315,7 @@
> SerialPortSetControl (
> //
> // Read the Modem Control Register.
> //
> - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET);
> + Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET);
> Mcr &= (~(MCR_DTRC | MCR_RTS));
>
> if ((Control & EFI_SERIAL_DATA_TERMINAL_READY) ==
> EFI_SERIAL_DATA_TERMINAL_READY) { @@ -267,7 +329,7 @@ SerialPortSetControl (
> //
> // Write the Modem Control Register.
> //
> - IoWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr);
> + SerialPortWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr);
>
> return RETURN_SUCCESS;
> }
> @@ -297,7 +359,7 @@ SerialPortGetControl (
> //
> // Read the Modem Status Register.
> //
> - Msr = IoRead8 ((UINT16) gUartBase + MSR_OFFSET);
> + Msr = SerialPortRead8 ((UINT16) gUartBase + MSR_OFFSET);
>
> if ((Msr & MSR_CTS) == MSR_CTS) {
> *Control |= EFI_SERIAL_CLEAR_TO_SEND; @@ -318,7 +380,7 @@
> SerialPortGetControl (
> //
> // Read the Modem Control Register.
> //
> - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET);
> + Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET);
>
> if ((Mcr & MCR_DTRC) == MCR_DTRC) {
> *Control |= EFI_SERIAL_DATA_TERMINAL_READY; @@ -331,7 +393,7 @@
> SerialPortGetControl (
> //
> // Read the Line Status Register.
> //
> - Lsr = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
> + Lsr = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
>
> if ((Lsr & LSR_TXRDY) == LSR_TXRDY) {
> *Control |= EFI_SERIAL_OUTPUT_BUFFER_EMPTY; @@ -470,19 +532,19 @@
> SerialPortSetAttributes (
> // Set communications format
> //
> OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (LcrParity
> << 3) | (LcrStop << 2) | LcrData);
> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>
> //
> // Configure baud rate
> //
> - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
> - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
> + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >>
> + 8));
> + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor &
> + 0xff));
>
> //
> // Switch back to bank 0
> //
> OutputData = (UINT8) ((gBreakSet << 6) | (LcrParity << 3) |
> (LcrStop << 2) | LcrData);
> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>
> return RETURN_SUCCESS;
> }
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib
2018-10-12 15:06 ` Sami Mujawar
@ 2018-10-12 15:31 ` Kinney, Michael D
2018-10-12 15:33 ` Ard Biesheuvel
1 sibling, 0 replies; 21+ messages in thread
From: Kinney, Michael D @ 2018-10-12 15:31 UTC (permalink / raw)
To: Sami Mujawar, Ard Biesheuvel, Kinney, Michael D
Cc: Ni, Ruiyu, nd, Stephanie Hughes-Fitt, edk2-devel@lists.01.org
Sami,
The BaseSerialPortLib16550 supports I/O port, MMIO, and PCI based UARTs.
PCDs are used to select the modes.
I would like to see the BaseSerialPortLib16550 used instead of
the serial port lib in PcAtChipsetPkg.
Mike
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org]
> On Behalf Of Sami Mujawar
> Sent: Friday, October 12, 2018 8:06 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; nd <nd@arm.com>;
> Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>;
> edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v1 1/6] PcAtChipsetPkg: Add
> MMIO Support to SerialIo Lib
>
> Hi Ard,
>
> I believe you are referring to
> MdeModulePkg\Library\BaseSerialPortLib16550\BaseSerialPor
> tLib16550.inf
> I have tried using this Serial Port Library. However,
> this has a dependency on PciLib which is difficult to
> resolve as we use the Serial port early in the boot.
> Please do let me know if you know a workaround to solve
> this issue.
>
> Regards,
>
> Sami Mujawar
>
> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: 12 October 2018 03:50 PM
> To: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: edk2-devel@lists.01.org; Leif Lindholm
> <leif.lindholm@linaro.org>; Ruiyu Ni
> <ruiyu.ni@intel.com>; Evan Lloyd <Evan.Lloyd@arm.com>;
> Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie
> Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd
> <nd@arm.com>
> Subject: Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO
> Support to SerialIo Lib
>
> On 12 October 2018 at 16:40, Sami Mujawar
> <sami.mujawar@arm.com> wrote:
> > Some virtual machine managers like kvmtool emulate the
> PC AT Serial
> > Port UART in the MMIO space so that architectures that
> do not support
> > I/O Mapped I/O can use the UART.
> >
> > This patch adds MMIO support to the PC AT
> SerialPortLib.
> >
> > PcdSerialUseMmio is used to select I/O or MMIO support.
> > If PcdSerialUseMmio is
> > TRUE - The value is read/written from MMIO space.
> > FALSE - The value is read/written from I/O space.
> > The Default is I/O space.
> >
>
> IIUC kvmtool exposes a 8250, and our 8250 driver already
> implements support for MMIO mapped registers. Can't you
> use that instead?
>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> > ---
> > The changes can be seen at
> >
> https://github.com/samimujawar/edk2/commit/84f908387d2031
> def5d374759e7
> > ad4cf90786c91
> >
> > Notes:
> > v1:
> > - Add support to read/write from UART registers
> using MMIO access
> > [SAMI]
> >
> > PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf |
> 4 +
> > PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c | 98
> > ++++++++++++++++----
> > 2 files changed, 84 insertions(+), 18 deletions(-)
> >
> > diff --git
> a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> > b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> > index
> >
> 959d6e27c9812d201d44d9249070ab7758cbfe00..d0689793040fd93
> 0701b02dae51f
> > f59ea16a10c4 100644
> > ---
> a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> > +++
> b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
> > @@ -2,6 +2,7 @@
> > # Library instance for SerialIo library class
> > #
> > # Copyright (c) 2006 - 2014, Intel Corporation. All
> rights
> > reserved.<BR>
> > +# Copyright (c) 2018, ARM Limited. All rights
> reserved.<BR>
> > # This program and the accompanying materials # are
> licensed and
> > made available under the terms and conditions of the
> BSD License #
> > which accompanies this distribution. The full text of
> the license may
> > be found at @@ -23,6 +24,7 @@ [Defines]
> >
> > [Packages]
> > MdePkg/MdePkg.dec
> > + MdeModulePkg/MdeModulePkg.dec
> >
> > [LibraryClasses]
> > BaseLib
> > @@ -31,3 +33,5 @@ [LibraryClasses]
> > [Sources]
> > SerialPortLib.c
> >
> > +[FixedPcd]
> > + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio
> ## CONSUMES
> > diff --git
> a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> > b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> > index
> >
> d1a1c6a03facad09e781b5605e22a24e5f51c618..898e5f957aae998
> 624189c6b5389
> > 12da0439dfe8 100644
> > ---
> a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> > +++
> b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
> > @@ -2,6 +2,8 @@
> > UART Serial Port library functions
> >
> > Copyright (c) 2006 - 2018, Intel Corporation. All
> rights
> > reserved.<BR>
> > + Copyright (c) 2018, ARM Limited. All rights
> reserved.<BR>
> > +
> > This program and the accompanying materials
> > are licensed and made available under the terms and
> conditions of the BSD License
> > which accompanies this distribution. The full text
> of the license may be found at
> > @@ -57,6 +59,66 @@ UINT8 gParity = 0;
> > UINT8 gBreakSet = 0;
> >
> > /**
> > + Reads an 8-bit value from the serial port.
> > +
> > + This function checks PcdSerialUseMmio to determine
> if I/O mapped
> > + IO or Memory Mapped IO operations must be performed
> on the serial
> > + port. It then uses the appropriate interface to read
> the Value from
> > + the serial port.
> > +
> > + If PcdSerialUseMmio is TRUE, then the value is read
> from MMIO space.
> > + If PcdSerialUseMmio is FALSE, then the value is read
> from I/O space.
> > +
> > + @param Address The UART register to read from.
> > +
> > + @return The value read from the serial port.
> > +
> > +**/
> > +STATIC
> > +UINT8
> > +SerialPortRead8 (
> > + IN UINTN Address
> > +)
> > +{
> > + if (FixedPcdGetBool (PcdSerialUseMmio)) {
> > + return MmioRead8 (Address);
> > + }
> > +
> > + return IoRead8 (Address);
> > +}
> > +
> > +/**
> > + Writes an 8-bit value to the serial port.
> > +
> > + This function checks PcdSerialUseMmio to determine
> if I/O mapped
> > + IO or Memory Mapped IO operations must be performed
> on the serial
> > + port. It then uses the appropriate interface to
> write the Value to
> > + the serial port.
> > +
> > + If PcdSerialUseMmio is TRUE, then the value is
> written to MMIO space.
> > + If PcdSerialUseMmio is FALSE, then the value is
> written to I/O space.
> > +
> > + @param Address The UART register to write.
> > + @param Value The value to write to the I/O
> port.
> > +
> > + @return The value written to the serial port.
> > +
> > +**/
> > +STATIC
> > +UINT8
> > +SerialPortWrite8 (
> > + IN UINTN Address,
> > + IN UINT8 Value
> > +)
> > +{
> > + if (FixedPcdGetBool (PcdSerialUseMmio)) {
> > + return MmioWrite8 (Address, Value);
> > + }
> > +
> > + return IoWrite8 (Address, Value);
> > +}
> > +
> > +/**
> > Initialize the serial device hardware.
> >
> > If no initialization is required, then return
> RETURN_SUCCESS.
> > @@ -91,19 +153,19 @@ SerialPortInitialize (
> > // Set communications format
> > //
> > OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6)
> | (gParity <<
> > 3) | (gStop << 2) | Data);
> > - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> > + SerialPortWrite8 (gUartBase + LCR_OFFSET,
> OutputData);
> >
> > //
> > // Configure baud rate
> > //
> > - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8)
> (Divisor >> 8));
> > - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8)
> (Divisor & 0xff));
> > + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET,
> (UINT8) (Divisor >>
> > + 8));
> > + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET,
> (UINT8) (Divisor &
> > + 0xff));
> >
> > //
> > // Switch back to bank 0
> > //
> > OutputData = (UINT8) ( (gBreakSet << 6) | (gParity
> << 3) | (gStop
> > << 2) | Data);
> > - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> > + SerialPortWrite8 (gUartBase + LCR_OFFSET,
> OutputData);
> >
> > return RETURN_SUCCESS;
> > }
> > @@ -148,9 +210,9 @@ SerialPortWrite (
> > // Wait for the serail port to be ready.
> > //
> > do {
> > - Data = IoRead8 ((UINT16) gUartBase +
> LSR_OFFSET);
> > + Data = SerialPortRead8 ((UINT16) gUartBase +
> LSR_OFFSET);
> > } while ((Data & LSR_TXRDY) == 0);
> > - IoWrite8 ((UINT16) gUartBase, *Buffer++);
> > + SerialPortWrite8 ((UINT16) gUartBase, *Buffer++);
> > }
> >
> > return Result;
> > @@ -189,10 +251,10 @@ SerialPortRead (
> > // Wait for the serail port to be ready.
> > //
> > do {
> > - Data = IoRead8 ((UINT16) gUartBase +
> LSR_OFFSET);
> > + Data = SerialPortRead8 ((UINT16) gUartBase +
> LSR_OFFSET);
> > } while ((Data & LSR_RXDA) == 0);
> >
> > - *Buffer++ = IoRead8 ((UINT16) gUartBase);
> > + *Buffer++ = SerialPortRead8 ((UINT16) gUartBase);
> > }
> >
> > return Result;
> > @@ -220,7 +282,7 @@ SerialPortPoll (
> > //
> > // Read the serial port status.
> > //
> > - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
> > + Data = SerialPortRead8 ((UINT16) gUartBase +
> LSR_OFFSET);
> >
> > return (BOOLEAN) ((Data & LSR_RXDA) != 0); } @@ -
> 253,7 +315,7 @@
> > SerialPortSetControl (
> > //
> > // Read the Modem Control Register.
> > //
> > - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET);
> > + Mcr = SerialPortRead8 ((UINT16) gUartBase +
> MCR_OFFSET);
> > Mcr &= (~(MCR_DTRC | MCR_RTS));
> >
> > if ((Control & EFI_SERIAL_DATA_TERMINAL_READY) ==
> > EFI_SERIAL_DATA_TERMINAL_READY) { @@ -267,7 +329,7 @@
> SerialPortSetControl (
> > //
> > // Write the Modem Control Register.
> > //
> > - IoWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr);
> > + SerialPortWrite8 ((UINT16) gUartBase + MCR_OFFSET,
> Mcr);
> >
> > return RETURN_SUCCESS;
> > }
> > @@ -297,7 +359,7 @@ SerialPortGetControl (
> > //
> > // Read the Modem Status Register.
> > //
> > - Msr = IoRead8 ((UINT16) gUartBase + MSR_OFFSET);
> > + Msr = SerialPortRead8 ((UINT16) gUartBase +
> MSR_OFFSET);
> >
> > if ((Msr & MSR_CTS) == MSR_CTS) {
> > *Control |= EFI_SERIAL_CLEAR_TO_SEND; @@ -318,7
> +380,7 @@
> > SerialPortGetControl (
> > //
> > // Read the Modem Control Register.
> > //
> > - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET);
> > + Mcr = SerialPortRead8 ((UINT16) gUartBase +
> MCR_OFFSET);
> >
> > if ((Mcr & MCR_DTRC) == MCR_DTRC) {
> > *Control |= EFI_SERIAL_DATA_TERMINAL_READY; @@ -
> 331,7 +393,7 @@
> > SerialPortGetControl (
> > //
> > // Read the Line Status Register.
> > //
> > - Lsr = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
> > + Lsr = SerialPortRead8 ((UINT16) gUartBase +
> LSR_OFFSET);
> >
> > if ((Lsr & LSR_TXRDY) == LSR_TXRDY) {
> > *Control |= EFI_SERIAL_OUTPUT_BUFFER_EMPTY; @@ -
> 470,19 +532,19 @@
> > SerialPortSetAttributes (
> > // Set communications format
> > //
> > OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6)
> | (LcrParity
> > << 3) | (LcrStop << 2) | LcrData);
> > - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> > + SerialPortWrite8 (gUartBase + LCR_OFFSET,
> OutputData);
> >
> > //
> > // Configure baud rate
> > //
> > - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8)
> (Divisor >> 8));
> > - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8)
> (Divisor & 0xff));
> > + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET,
> (UINT8) (Divisor >>
> > + 8));
> > + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET,
> (UINT8) (Divisor &
> > + 0xff));
> >
> > //
> > // Switch back to bank 0
> > //
> > OutputData = (UINT8) ((gBreakSet << 6) | (LcrParity
> << 3) |
> > (LcrStop << 2) | LcrData);
> > - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
> > + SerialPortWrite8 (gUartBase + LCR_OFFSET,
> OutputData);
> >
> > return RETURN_SUCCESS;
> > }
> > --
> > 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> >
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib
2018-10-12 15:06 ` Sami Mujawar
2018-10-12 15:31 ` Kinney, Michael D
@ 2018-10-12 15:33 ` Ard Biesheuvel
2018-10-15 2:38 ` Ni, Ruiyu
1 sibling, 1 reply; 21+ messages in thread
From: Ard Biesheuvel @ 2018-10-12 15:33 UTC (permalink / raw)
To: Sami Mujawar
Cc: edk2-devel@lists.01.org, Leif Lindholm, Ruiyu Ni, Evan Lloyd,
Matteo Carlini, Stephanie Hughes-Fitt, nd
On 12 October 2018 at 17:06, Sami Mujawar <Sami.Mujawar@arm.com> wrote:
> Hi Ard,
>
> I believe you are referring to MdeModulePkg\Library\BaseSerialPortLib16550\BaseSerialPortLib16550.inf
> I have tried using this Serial Port Library. However, this has a dependency on PciLib which is difficult to resolve as we use the Serial port early in the boot.
> Please do let me know if you know a workaround to solve this issue.
>
Yes. Just keep PcdSerialPciDeviceInfo at its default of 0xff, and the
PCI dependencies in that driver are essentially circumvented, so you
can use any PciLib implementation as a dummy (or create a new one that
ASSERT()s in all functions if you prefer)
> Regards,
>
> Sami Mujawar
>
> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: 12 October 2018 03:50 PM
> To: Sami Mujawar <Sami.Mujawar@arm.com>
> Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>; Ruiyu Ni <ruiyu.ni@intel.com>; Evan Lloyd <Evan.Lloyd@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; Stephanie Hughes-Fitt <Stephanie.Hughes-Fitt@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib
>
> On 12 October 2018 at 16:40, Sami Mujawar <sami.mujawar@arm.com> wrote:
>> Some virtual machine managers like kvmtool emulate the PC AT Serial
>> Port UART in the MMIO space so that architectures that do not support
>> I/O Mapped I/O can use the UART.
>>
>> This patch adds MMIO support to the PC AT SerialPortLib.
>>
>> PcdSerialUseMmio is used to select I/O or MMIO support.
>> If PcdSerialUseMmio is
>> TRUE - The value is read/written from MMIO space.
>> FALSE - The value is read/written from I/O space.
>> The Default is I/O space.
>>
>
> IIUC kvmtool exposes a 8250, and our 8250 driver already implements support for MMIO mapped registers. Can't you use that instead?
>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>> ---
>> The changes can be seen at
>> https://github.com/samimujawar/edk2/commit/84f908387d2031def5d374759e7
>> ad4cf90786c91
>>
>> Notes:
>> v1:
>> - Add support to read/write from UART registers using MMIO access
>> [SAMI]
>>
>> PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf | 4 +
>> PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c | 98
>> ++++++++++++++++----
>> 2 files changed, 84 insertions(+), 18 deletions(-)
>>
>> diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> index
>> 959d6e27c9812d201d44d9249070ab7758cbfe00..d0689793040fd930701b02dae51f
>> f59ea16a10c4 100644
>> --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
>> @@ -2,6 +2,7 @@
>> # Library instance for SerialIo library class
>> #
>> # Copyright (c) 2006 - 2014, Intel Corporation. All rights
>> reserved.<BR>
>> +# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
>> # This program and the accompanying materials # are licensed and
>> made available under the terms and conditions of the BSD License #
>> which accompanies this distribution. The full text of the license may
>> be found at @@ -23,6 +24,7 @@ [Defines]
>>
>> [Packages]
>> MdePkg/MdePkg.dec
>> + MdeModulePkg/MdeModulePkg.dec
>>
>> [LibraryClasses]
>> BaseLib
>> @@ -31,3 +33,5 @@ [LibraryClasses]
>> [Sources]
>> SerialPortLib.c
>>
>> +[FixedPcd]
>> + gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio ## CONSUMES
>> diff --git a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
>> b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
>> index
>> d1a1c6a03facad09e781b5605e22a24e5f51c618..898e5f957aae998624189c6b5389
>> 12da0439dfe8 100644
>> --- a/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
>> +++ b/PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c
>> @@ -2,6 +2,8 @@
>> UART Serial Port library functions
>>
>> Copyright (c) 2006 - 2018, Intel Corporation. All rights
>> reserved.<BR>
>> + Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
>> +
>> This program and the accompanying materials
>> are licensed and made available under the terms and conditions of the BSD License
>> which accompanies this distribution. The full text of the license may be found at
>> @@ -57,6 +59,66 @@ UINT8 gParity = 0;
>> UINT8 gBreakSet = 0;
>>
>> /**
>> + Reads an 8-bit value from the serial port.
>> +
>> + This function checks PcdSerialUseMmio to determine if I/O mapped
>> + IO or Memory Mapped IO operations must be performed on the serial
>> + port. It then uses the appropriate interface to read the Value from
>> + the serial port.
>> +
>> + If PcdSerialUseMmio is TRUE, then the value is read from MMIO space.
>> + If PcdSerialUseMmio is FALSE, then the value is read from I/O space.
>> +
>> + @param Address The UART register to read from.
>> +
>> + @return The value read from the serial port.
>> +
>> +**/
>> +STATIC
>> +UINT8
>> +SerialPortRead8 (
>> + IN UINTN Address
>> +)
>> +{
>> + if (FixedPcdGetBool (PcdSerialUseMmio)) {
>> + return MmioRead8 (Address);
>> + }
>> +
>> + return IoRead8 (Address);
>> +}
>> +
>> +/**
>> + Writes an 8-bit value to the serial port.
>> +
>> + This function checks PcdSerialUseMmio to determine if I/O mapped
>> + IO or Memory Mapped IO operations must be performed on the serial
>> + port. It then uses the appropriate interface to write the Value to
>> + the serial port.
>> +
>> + If PcdSerialUseMmio is TRUE, then the value is written to MMIO space.
>> + If PcdSerialUseMmio is FALSE, then the value is written to I/O space.
>> +
>> + @param Address The UART register to write.
>> + @param Value The value to write to the I/O port.
>> +
>> + @return The value written to the serial port.
>> +
>> +**/
>> +STATIC
>> +UINT8
>> +SerialPortWrite8 (
>> + IN UINTN Address,
>> + IN UINT8 Value
>> +)
>> +{
>> + if (FixedPcdGetBool (PcdSerialUseMmio)) {
>> + return MmioWrite8 (Address, Value);
>> + }
>> +
>> + return IoWrite8 (Address, Value);
>> +}
>> +
>> +/**
>> Initialize the serial device hardware.
>>
>> If no initialization is required, then return RETURN_SUCCESS.
>> @@ -91,19 +153,19 @@ SerialPortInitialize (
>> // Set communications format
>> //
>> OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (gParity <<
>> 3) | (gStop << 2) | Data);
>> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
>> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>>
>> //
>> // Configure baud rate
>> //
>> - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
>> - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
>> + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >>
>> + 8));
>> + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor &
>> + 0xff));
>>
>> //
>> // Switch back to bank 0
>> //
>> OutputData = (UINT8) ( (gBreakSet << 6) | (gParity << 3) | (gStop
>> << 2) | Data);
>> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
>> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>>
>> return RETURN_SUCCESS;
>> }
>> @@ -148,9 +210,9 @@ SerialPortWrite (
>> // Wait for the serail port to be ready.
>> //
>> do {
>> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
>> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
>> } while ((Data & LSR_TXRDY) == 0);
>> - IoWrite8 ((UINT16) gUartBase, *Buffer++);
>> + SerialPortWrite8 ((UINT16) gUartBase, *Buffer++);
>> }
>>
>> return Result;
>> @@ -189,10 +251,10 @@ SerialPortRead (
>> // Wait for the serail port to be ready.
>> //
>> do {
>> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
>> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
>> } while ((Data & LSR_RXDA) == 0);
>>
>> - *Buffer++ = IoRead8 ((UINT16) gUartBase);
>> + *Buffer++ = SerialPortRead8 ((UINT16) gUartBase);
>> }
>>
>> return Result;
>> @@ -220,7 +282,7 @@ SerialPortPoll (
>> //
>> // Read the serial port status.
>> //
>> - Data = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
>> + Data = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
>>
>> return (BOOLEAN) ((Data & LSR_RXDA) != 0); } @@ -253,7 +315,7 @@
>> SerialPortSetControl (
>> //
>> // Read the Modem Control Register.
>> //
>> - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET);
>> + Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET);
>> Mcr &= (~(MCR_DTRC | MCR_RTS));
>>
>> if ((Control & EFI_SERIAL_DATA_TERMINAL_READY) ==
>> EFI_SERIAL_DATA_TERMINAL_READY) { @@ -267,7 +329,7 @@ SerialPortSetControl (
>> //
>> // Write the Modem Control Register.
>> //
>> - IoWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr);
>> + SerialPortWrite8 ((UINT16) gUartBase + MCR_OFFSET, Mcr);
>>
>> return RETURN_SUCCESS;
>> }
>> @@ -297,7 +359,7 @@ SerialPortGetControl (
>> //
>> // Read the Modem Status Register.
>> //
>> - Msr = IoRead8 ((UINT16) gUartBase + MSR_OFFSET);
>> + Msr = SerialPortRead8 ((UINT16) gUartBase + MSR_OFFSET);
>>
>> if ((Msr & MSR_CTS) == MSR_CTS) {
>> *Control |= EFI_SERIAL_CLEAR_TO_SEND; @@ -318,7 +380,7 @@
>> SerialPortGetControl (
>> //
>> // Read the Modem Control Register.
>> //
>> - Mcr = IoRead8 ((UINT16) gUartBase + MCR_OFFSET);
>> + Mcr = SerialPortRead8 ((UINT16) gUartBase + MCR_OFFSET);
>>
>> if ((Mcr & MCR_DTRC) == MCR_DTRC) {
>> *Control |= EFI_SERIAL_DATA_TERMINAL_READY; @@ -331,7 +393,7 @@
>> SerialPortGetControl (
>> //
>> // Read the Line Status Register.
>> //
>> - Lsr = IoRead8 ((UINT16) gUartBase + LSR_OFFSET);
>> + Lsr = SerialPortRead8 ((UINT16) gUartBase + LSR_OFFSET);
>>
>> if ((Lsr & LSR_TXRDY) == LSR_TXRDY) {
>> *Control |= EFI_SERIAL_OUTPUT_BUFFER_EMPTY; @@ -470,19 +532,19 @@
>> SerialPortSetAttributes (
>> // Set communications format
>> //
>> OutputData = (UINT8) ((DLAB << 7) | (gBreakSet << 6) | (LcrParity
>> << 3) | (LcrStop << 2) | LcrData);
>> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
>> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>>
>> //
>> // Configure baud rate
>> //
>> - IoWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >> 8));
>> - IoWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor & 0xff));
>> + SerialPortWrite8 (gUartBase + BAUD_HIGH_OFFSET, (UINT8) (Divisor >>
>> + 8));
>> + SerialPortWrite8 (gUartBase + BAUD_LOW_OFFSET, (UINT8) (Divisor &
>> + 0xff));
>>
>> //
>> // Switch back to bank 0
>> //
>> OutputData = (UINT8) ((gBreakSet << 6) | (LcrParity << 3) |
>> (LcrStop << 2) | LcrData);
>> - IoWrite8 (gUartBase + LCR_OFFSET, OutputData);
>> + SerialPortWrite8 (gUartBase + LCR_OFFSET, OutputData);
>>
>> return RETURN_SUCCESS;
>> }
>> --
>> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>>
>>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib
2018-10-12 15:33 ` Ard Biesheuvel
@ 2018-10-15 2:38 ` Ni, Ruiyu
0 siblings, 0 replies; 21+ messages in thread
From: Ni, Ruiyu @ 2018-10-15 2:38 UTC (permalink / raw)
To: Ard Biesheuvel, Sami Mujawar
Cc: edk2-devel@lists.01.org, Leif Lindholm, Evan Lloyd,
Matteo Carlini, Stephanie Hughes-Fitt, nd
On 10/12/2018 11:33 PM, Ard Biesheuvel wrote:
> On 12 October 2018 at 17:06, Sami Mujawar <Sami.Mujawar@arm.com> wrote:
>> Hi Ard,
>>
>> I believe you are referring to MdeModulePkg\Library\BaseSerialPortLib16550\BaseSerialPortLib16550.inf
>> I have tried using this Serial Port Library. However, this has a dependency on PciLib which is difficult to resolve as we use the Serial port early in the boot.
>> Please do let me know if you know a workaround to solve this issue.
>>
>
> Yes. Just keep PcdSerialPciDeviceInfo at its default of 0xff, and the
> PCI dependencies in that driver are essentially circumvented, so you
> can use any PciLib implementation as a dummy (or create a new one that
> ASSERT()s in all functions if you prefer)
>
Yes I was just about to say so. Actually I plan to retire/remove the
SerialIoLib in PcAtChipsetPkg.
--
Thanks,
Ray
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 2/6] PcAtChipsetPkg: Add MMIO Support to RTC driver
2018-10-12 14:40 [PATCH 0/6] Add kvmtool emulated platform support for ARM Sami Mujawar
2018-10-12 14:40 ` [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib Sami Mujawar
@ 2018-10-12 14:40 ` Sami Mujawar
2018-10-13 10:51 ` Leif Lindholm
2018-10-12 14:40 ` [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory Sami Mujawar
` (4 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Sami Mujawar @ 2018-10-12 14:40 UTC (permalink / raw)
To: edk2-devel
Cc: ard.biesheuvel, leif.lindholm, ruiyu.ni, evan.lloyd,
Matteo.Carlini, Stephanie.Hughes-Fitt, nd
Some virtual machine managers like kvmtool emulate the MC146818
RTC controller in the MMIO space so that architectures that do
not support I/O Mapped I/O can use the RTC. This patch adds MMIO
support to the RTC controller driver.
The PCD PcdRtcUseMmio has been added to select I/O or MMIO support.
If PcdRtcUseMmio is:
TRUE - Indicates the RTC port registers are in MMIO space.
FALSE - Indicates the RTC port registers are in I/O space.
Default is I/O space.
When MMIO support is selected (PcdRtcUseMmio == TRUE) the driver
maps the MMIO region used by the RTC as runtime memory so that the
RTC registers are accessible post ExitBootServices.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
The changes can be seen at https://github.com/samimujawar/edk2/commit/d9fa7df2204c12883a795c7df429578c310725bf
Notes:
v1:
- Add support to read/write from RTC registers using MMIO access [SAMI]
PcAtChipsetPkg/PcAtChipsetPkg.dec | 8 ++
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 38 ++++++-
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c | 112 +++++++++++++++++++-
PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf | 8 ++
4 files changed, 162 insertions(+), 4 deletions(-)
diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec b/PcAtChipsetPkg/PcAtChipsetPkg.dec
index 0e66ff0ba3770cf765cfe36d592151d72eaa238a..5aaf5e73811c4dba3b83768f1535e028938ead1f 100644
--- a/PcAtChipsetPkg/PcAtChipsetPkg.dec
+++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec
@@ -6,6 +6,7 @@
#
# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -202,5 +203,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
# @Prompt RTC Target Register address
gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister|0x71|UINT8|0x0000001F
+ ## Indicates the RTC port registers are in MMIO space, or in I/O space.
+ # Default is I/O space.<BR><BR>
+ # TRUE - RTC port registers are in MMIO space.<BR>
+ # FALSE - RTC port registers are in I/O space.<BR>
+ # @Prompt RTC port registers use MMIO.
+ gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|FALSE|BOOLEAN|0x00000020
+
[UserExtensions.TianoCore."ExtraFiles"]
PcAtChipsetPkgExtra.uni
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
index 7965eb8aa55b92caef7a63834695506123935e2d..ee746add57b9778bf09a4d9eb6881d06caa14aba 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
@@ -3,6 +3,7 @@
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
@@ -16,6 +17,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include "PcRtc.h"
+extern EFI_PHYSICAL_ADDRESS mRtcRegisterBase;
+
//
// Days of month.
//
@@ -72,7 +75,21 @@ RtcRead (
IN UINT8 Address
)
{
- IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
+ if (FixedPcdGetBool (PcdRtcUseMmio)) {
+ MmioWrite8 (
+ mRtcRegisterBase,
+ (UINT8)(Address | (UINT8)(MmioRead8 (mRtcRegisterBase) & 0x80))
+ );
+ return MmioRead8 (
+ mRtcRegisterBase + (PcdGet8 (PcdRtcTargetRegister) -
+ PcdGet8 (PcdRtcIndexRegister))
+ );
+ }
+
+ IoWrite8 (
+ PcdGet8 (PcdRtcIndexRegister),
+ (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
+ );
return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
}
@@ -90,8 +107,23 @@ RtcWrite (
IN UINT8 Data
)
{
- IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
- IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
+ if (FixedPcdGetBool (PcdRtcUseMmio)) {
+ MmioWrite8 (
+ mRtcRegisterBase,
+ (UINT8)(Address | (UINT8)(MmioRead8 (mRtcRegisterBase) & 0x80))
+ );
+ MmioWrite8 (
+ mRtcRegisterBase + (PcdGet8 (PcdRtcTargetRegister) -
+ PcdGet8 (PcdRtcIndexRegister)),
+ Data
+ );
+ } else {
+ IoWrite8 (
+ PcdGet8 (PcdRtcIndexRegister),
+ (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
+ );
+ IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
+ }
}
/**
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
index 56ddc3ed5b1accc2b1a99ab4515f851f89fb6e43..d6d7251309001a203282eb7022cc71ac4f0ef62e 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
@@ -2,6 +2,7 @@
Provides Set/Get time operations.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -12,12 +13,17 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
+#include <Library/DxeServicesTableLib.h>
#include "PcRtc.h"
PC_RTC_MODULE_GLOBALS mModuleGlobal;
EFI_HANDLE mHandle = NULL;
+STATIC EFI_EVENT mVirtualAddrChangeEvent;
+
+EFI_PHYSICAL_ADDRESS mRtcRegisterBase;
+
/**
Returns the current time and date information, and the time-keeping capabilities
of the hardware platform.
@@ -112,6 +118,29 @@ PcRtcEfiSetWakeupTime (
}
/**
+ Fixup internal data so that EFI can be called in virtual mode.
+ Call the passed in Child Notify event and convert any pointers in
+ lib to virtual mode.
+
+ @param[in] Event The Event that is being processed
+ @param[in] Context Event Context
+**/
+VOID
+EFIAPI
+LibRtcVirtualNotifyEvent (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ // Only needed if you are going to support the OS calling RTC functions in
+ // virtual mode. You will need to call EfiConvertPointer (). To convert any
+ // stored physical addresses to virtual address. After the OS transitions to
+ // calling in virtual mode, all future runtime calls will be made in virtual
+ // mode.
+ EfiConvertPointer (0x0, (VOID**)&mRtcRegisterBase);
+}
+
+/**
The user Entry Point for PcRTC module.
This is the entrhy point for PcRTC module. It installs the UEFI runtime service
@@ -133,10 +162,75 @@ InitializePcRtc (
{
EFI_STATUS Status;
EFI_EVENT Event;
+ EFI_PHYSICAL_ADDRESS RtcPageBase;
EfiInitializeLock (&mModuleGlobal.RtcLock, TPL_CALLBACK);
mModuleGlobal.CenturyRtcAddress = GetCenturyRtcAddress ();
+ if (FixedPcdGetBool (PcdRtcUseMmio)) {
+ mRtcRegisterBase = PcdGet8 (PcdRtcIndexRegister);
+ RtcPageBase = mRtcRegisterBase & ~(EFI_PAGE_SIZE - 1);
+
+ // Declare the controller as EFI_MEMORY_RUNTIME
+ Status = gDS->AddMemorySpace (
+ EfiGcdMemoryTypeMemoryMappedIo,
+ RtcPageBase,
+ EFI_PAGE_SIZE,
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR, "Failed to add memory space. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ Status = gDS->AllocateMemorySpace (
+ EfiGcdAllocateAddress,
+ EfiGcdMemoryTypeMemoryMappedIo,
+ 0,
+ EFI_PAGE_SIZE,
+ &RtcPageBase,
+ ImageHandle,
+ NULL
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to allocate memory space. Status = %r\n",
+ Status
+ ));
+ gDS->RemoveMemorySpace (
+ RtcPageBase,
+ EFI_PAGE_SIZE
+ );
+ return Status;
+ }
+
+ Status = gDS->SetMemorySpaceAttributes (
+ RtcPageBase,
+ EFI_PAGE_SIZE,
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to set memory attributes. Status = %r\n",
+ Status
+ ));
+ gDS->FreeMemorySpace (
+ RtcPageBase,
+ EFI_PAGE_SIZE
+ );
+ gDS->RemoveMemorySpace (
+ RtcPageBase,
+ EFI_PAGE_SIZE
+ );
+ return Status;
+ }
+ }
+
Status = PcRtcInit (&mModuleGlobal);
ASSERT_EFI_ERROR (Status);
@@ -171,7 +265,23 @@ InitializePcRtc (
NULL,
NULL
);
- ASSERT_EFI_ERROR (Status);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
+
+ if (FixedPcdGetBool (PcdRtcUseMmio)) {
+ // Register for the virtual address change event
+ Status = gBS->CreateEventEx (
+ EVT_NOTIFY_SIGNAL,
+ TPL_NOTIFY,
+ LibRtcVirtualNotifyEvent,
+ NULL,
+ &gEfiEventVirtualAddressChangeGuid,
+ &mVirtualAddrChangeEvent
+ );
+ ASSERT_EFI_ERROR (Status);
+ }
return Status;
}
diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
index 876298e1e0f3745cd3213509ffed4e091fbedef8..2dee5a77cfc23878a64a809bcb64e3745230ea5a 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
@@ -6,6 +6,7 @@
#
# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
# Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
+# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -55,6 +56,7 @@ [LibraryClasses]
BaseLib
PcdLib
ReportStatusCodeLib
+ DxeServicesTableLib
[Protocols]
gEfiRealTimeClockArchProtocolGuid ## PRODUCES
@@ -68,10 +70,13 @@ [Guids]
## SOMETIMES_CONSUMES ## SystemTable
gEfiAcpiTableGuid
+ gEfiEventVirtualAddressChangeGuid
+
[FixedPcd]
gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterA ## CONSUMES
gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterB ## CONSUMES
gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterD ## CONSUMES
+ gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio ## CONSUMES
[Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdRealTimeClockUpdateTimeout ## CONSUMES
@@ -83,5 +88,8 @@ [Pcd]
[Depex]
gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid
+[Depex.common.DXE_RUNTIME_DRIVER]
+ gEfiCpuArchProtocolGuid
+
[UserExtensions.TianoCore."ExtraFiles"]
PcRtcExtra.uni
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 2/6] PcAtChipsetPkg: Add MMIO Support to RTC driver
2018-10-12 14:40 ` [PATCH v1 2/6] PcAtChipsetPkg: Add MMIO Support to RTC driver Sami Mujawar
@ 2018-10-13 10:51 ` Leif Lindholm
0 siblings, 0 replies; 21+ messages in thread
From: Leif Lindholm @ 2018-10-13 10:51 UTC (permalink / raw)
To: Sami Mujawar
Cc: edk2-devel, ard.biesheuvel, ruiyu.ni, evan.lloyd, Matteo.Carlini,
Stephanie.Hughes-Fitt, nd
On Fri, Oct 12, 2018 at 03:40:05PM +0100, Sami Mujawar wrote:
> Some virtual machine managers like kvmtool emulate the MC146818
> RTC controller in the MMIO space so that architectures that do
> not support I/O Mapped I/O can use the RTC. This patch adds MMIO
> support to the RTC controller driver.
>
> The PCD PcdRtcUseMmio has been added to select I/O or MMIO support.
> If PcdRtcUseMmio is:
> TRUE - Indicates the RTC port registers are in MMIO space.
> FALSE - Indicates the RTC port registers are in I/O space.
> Default is I/O space.
>
> When MMIO support is selected (PcdRtcUseMmio == TRUE) the driver
> maps the MMIO region used by the RTC as runtime memory so that the
> RTC registers are accessible post ExitBootServices.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> The changes can be seen at https://github.com/samimujawar/edk2/commit/d9fa7df2204c12883a795c7df429578c310725bf
>
> Notes:
> v1:
> - Add support to read/write from RTC registers using MMIO access [SAMI]
>
> PcAtChipsetPkg/PcAtChipsetPkg.dec | 8 ++
> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 38 ++++++-
> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c | 112 +++++++++++++++++++-
> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf | 8 ++
> 4 files changed, 162 insertions(+), 4 deletions(-)
>
> diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dec b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> index 0e66ff0ba3770cf765cfe36d592151d72eaa238a..5aaf5e73811c4dba3b83768f1535e028938ead1f 100644
> --- a/PcAtChipsetPkg/PcAtChipsetPkg.dec
> +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dec
> @@ -6,6 +6,7 @@
> #
> # Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.<BR>
> # Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
> #
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
> @@ -202,5 +203,12 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
> # @Prompt RTC Target Register address
> gPcAtChipsetPkgTokenSpaceGuid.PcdRtcTargetRegister|0x71|UINT8|0x0000001F
>
> + ## Indicates the RTC port registers are in MMIO space, or in I/O space.
> + # Default is I/O space.<BR><BR>
> + # TRUE - RTC port registers are in MMIO space.<BR>
> + # FALSE - RTC port registers are in I/O space.<BR>
> + # @Prompt RTC port registers use MMIO.
> + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|FALSE|BOOLEAN|0x00000020
> +
> [UserExtensions.TianoCore."ExtraFiles"]
> PcAtChipsetPkgExtra.uni
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> index 7965eb8aa55b92caef7a63834695506123935e2d..ee746add57b9778bf09a4d9eb6881d06caa14aba 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
> @@ -3,6 +3,7 @@
>
> Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> @@ -16,6 +17,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> #include "PcRtc.h"
>
> +extern EFI_PHYSICAL_ADDRESS mRtcRegisterBase;
> +
> //
> // Days of month.
> //
> @@ -72,7 +75,21 @@ RtcRead (
> IN UINT8 Address
> )
> {
> - IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
> + if (FixedPcdGetBool (PcdRtcUseMmio)) {
> + MmioWrite8 (
> + mRtcRegisterBase,
> + (UINT8)(Address | (UINT8)(MmioRead8 (mRtcRegisterBase) & 0x80))
> + );
> + return MmioRead8 (
> + mRtcRegisterBase + (PcdGet8 (PcdRtcTargetRegister) -
> + PcdGet8 (PcdRtcIndexRegister))
> + );
> + }
> +
> + IoWrite8 (
> + PcdGet8 (PcdRtcIndexRegister),
> + (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
> + );
> return IoRead8 (PcdGet8 (PcdRtcTargetRegister));
So, this isn't my package, but I would much rather see this solved
with local (STATIC) wrapper functions (i.e. RtcRead*/RtcWrite*) for
the accessors instead of conditionalising which one to use at every
call site.
/
Leif
> }
>
> @@ -90,8 +107,23 @@ RtcWrite (
> IN UINT8 Data
> )
> {
> - IoWrite8 (PcdGet8 (PcdRtcIndexRegister), (UINT8) (Address | (UINT8) (IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80)));
> - IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
> + if (FixedPcdGetBool (PcdRtcUseMmio)) {
> + MmioWrite8 (
> + mRtcRegisterBase,
> + (UINT8)(Address | (UINT8)(MmioRead8 (mRtcRegisterBase) & 0x80))
> + );
> + MmioWrite8 (
> + mRtcRegisterBase + (PcdGet8 (PcdRtcTargetRegister) -
> + PcdGet8 (PcdRtcIndexRegister)),
> + Data
> + );
> + } else {
> + IoWrite8 (
> + PcdGet8 (PcdRtcIndexRegister),
> + (UINT8)(Address | (UINT8)(IoRead8 (PcdGet8 (PcdRtcIndexRegister)) & 0x80))
> + );
> + IoWrite8 (PcdGet8 (PcdRtcTargetRegister), Data);
> + }
> }
>
> /**
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> index 56ddc3ed5b1accc2b1a99ab4515f851f89fb6e43..d6d7251309001a203282eb7022cc71ac4f0ef62e 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c
> @@ -2,6 +2,7 @@
> Provides Set/Get time operations.
>
> Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
> This program and the accompanying materials
> are licensed and made available under the terms and conditions of the BSD License
> which accompanies this distribution. The full text of the license may be found at
> @@ -12,12 +13,17 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> **/
>
> +#include <Library/DxeServicesTableLib.h>
> #include "PcRtc.h"
>
> PC_RTC_MODULE_GLOBALS mModuleGlobal;
>
> EFI_HANDLE mHandle = NULL;
>
> +STATIC EFI_EVENT mVirtualAddrChangeEvent;
> +
> +EFI_PHYSICAL_ADDRESS mRtcRegisterBase;
> +
> /**
> Returns the current time and date information, and the time-keeping capabilities
> of the hardware platform.
> @@ -112,6 +118,29 @@ PcRtcEfiSetWakeupTime (
> }
>
> /**
> + Fixup internal data so that EFI can be called in virtual mode.
> + Call the passed in Child Notify event and convert any pointers in
> + lib to virtual mode.
> +
> + @param[in] Event The Event that is being processed
> + @param[in] Context Event Context
> +**/
> +VOID
> +EFIAPI
> +LibRtcVirtualNotifyEvent (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + // Only needed if you are going to support the OS calling RTC functions in
> + // virtual mode. You will need to call EfiConvertPointer (). To convert any
> + // stored physical addresses to virtual address. After the OS transitions to
> + // calling in virtual mode, all future runtime calls will be made in virtual
> + // mode.
> + EfiConvertPointer (0x0, (VOID**)&mRtcRegisterBase);
> +}
> +
> +/**
> The user Entry Point for PcRTC module.
>
> This is the entrhy point for PcRTC module. It installs the UEFI runtime service
> @@ -133,10 +162,75 @@ InitializePcRtc (
> {
> EFI_STATUS Status;
> EFI_EVENT Event;
> + EFI_PHYSICAL_ADDRESS RtcPageBase;
>
> EfiInitializeLock (&mModuleGlobal.RtcLock, TPL_CALLBACK);
> mModuleGlobal.CenturyRtcAddress = GetCenturyRtcAddress ();
>
> + if (FixedPcdGetBool (PcdRtcUseMmio)) {
> + mRtcRegisterBase = PcdGet8 (PcdRtcIndexRegister);
> + RtcPageBase = mRtcRegisterBase & ~(EFI_PAGE_SIZE - 1);
> +
> + // Declare the controller as EFI_MEMORY_RUNTIME
> + Status = gDS->AddMemorySpace (
> + EfiGcdMemoryTypeMemoryMappedIo,
> + RtcPageBase,
> + EFI_PAGE_SIZE,
> + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR, "Failed to add memory space. Status = %r\n",
> + Status
> + ));
> + return Status;
> + }
> +
> + Status = gDS->AllocateMemorySpace (
> + EfiGcdAllocateAddress,
> + EfiGcdMemoryTypeMemoryMappedIo,
> + 0,
> + EFI_PAGE_SIZE,
> + &RtcPageBase,
> + ImageHandle,
> + NULL
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "Failed to allocate memory space. Status = %r\n",
> + Status
> + ));
> + gDS->RemoveMemorySpace (
> + RtcPageBase,
> + EFI_PAGE_SIZE
> + );
> + return Status;
> + }
> +
> + Status = gDS->SetMemorySpaceAttributes (
> + RtcPageBase,
> + EFI_PAGE_SIZE,
> + EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "Failed to set memory attributes. Status = %r\n",
> + Status
> + ));
> + gDS->FreeMemorySpace (
> + RtcPageBase,
> + EFI_PAGE_SIZE
> + );
> + gDS->RemoveMemorySpace (
> + RtcPageBase,
> + EFI_PAGE_SIZE
> + );
> + return Status;
> + }
> + }
> +
> Status = PcRtcInit (&mModuleGlobal);
> ASSERT_EFI_ERROR (Status);
>
> @@ -171,7 +265,23 @@ InitializePcRtc (
> NULL,
> NULL
> );
> - ASSERT_EFI_ERROR (Status);
> + if (EFI_ERROR (Status)) {
> + ASSERT_EFI_ERROR (Status);
> + return Status;
> + }
> +
> + if (FixedPcdGetBool (PcdRtcUseMmio)) {
> + // Register for the virtual address change event
> + Status = gBS->CreateEventEx (
> + EVT_NOTIFY_SIGNAL,
> + TPL_NOTIFY,
> + LibRtcVirtualNotifyEvent,
> + NULL,
> + &gEfiEventVirtualAddressChangeGuid,
> + &mVirtualAddrChangeEvent
> + );
> + ASSERT_EFI_ERROR (Status);
> + }
>
> return Status;
> }
> diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
> index 876298e1e0f3745cd3213509ffed4e091fbedef8..2dee5a77cfc23878a64a809bcb64e3745230ea5a 100644
> --- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
> +++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
> @@ -6,6 +6,7 @@
> #
> # Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> # Copyright (c) 2017, AMD Inc. All rights reserved.<BR>
> +# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
> #
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD License
> @@ -55,6 +56,7 @@ [LibraryClasses]
> BaseLib
> PcdLib
> ReportStatusCodeLib
> + DxeServicesTableLib
>
> [Protocols]
> gEfiRealTimeClockArchProtocolGuid ## PRODUCES
> @@ -68,10 +70,13 @@ [Guids]
> ## SOMETIMES_CONSUMES ## SystemTable
> gEfiAcpiTableGuid
>
> + gEfiEventVirtualAddressChangeGuid
> +
> [FixedPcd]
> gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterA ## CONSUMES
> gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterB ## CONSUMES
> gPcAtChipsetPkgTokenSpaceGuid.PcdInitialValueRtcRegisterD ## CONSUMES
> + gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio ## CONSUMES
>
> [Pcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdRealTimeClockUpdateTimeout ## CONSUMES
> @@ -83,5 +88,8 @@ [Pcd]
> [Depex]
> gEfiVariableArchProtocolGuid AND gEfiVariableWriteArchProtocolGuid
>
> +[Depex.common.DXE_RUNTIME_DRIVER]
> + gEfiCpuArchProtocolGuid
> +
> [UserExtensions.TianoCore."ExtraFiles"]
> PcRtcExtra.uni
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory
2018-10-12 14:40 [PATCH 0/6] Add kvmtool emulated platform support for ARM Sami Mujawar
2018-10-12 14:40 ` [PATCH v1 1/6] PcAtChipsetPkg: Add MMIO Support to SerialIo Lib Sami Mujawar
2018-10-12 14:40 ` [PATCH v1 2/6] PcAtChipsetPkg: Add MMIO Support to RTC driver Sami Mujawar
@ 2018-10-12 14:40 ` Sami Mujawar
2018-10-13 9:09 ` Zeng, Star
2018-10-13 21:28 ` Laszlo Ersek
2018-10-12 14:40 ` [PATCH v1 4/6] ArmVirtPkg: Save DT base address from X0 in PCD Sami Mujawar
` (3 subsequent siblings)
6 siblings, 2 replies; 21+ messages in thread
From: Sami Mujawar @ 2018-10-12 14:40 UTC (permalink / raw)
To: edk2-devel
Cc: ard.biesheuvel, leif.lindholm, star.zeng, jian.j.wang, ruiyu.ni,
evan.lloyd, Matteo.Carlini, Stephanie.Hughes-Fitt, nd
Some platforms are able to preserve a memory range across
system resets. This memory can be used for the Non-Volatile
variables storage. The PcdEmuVariableNvStoreReserved is
used to specify this option.
This patch enables mapping of this Non-Volatile memory range
as runtime memory so that the variable storage is accessible
post ExitBootServices.
Also added PcdMapEmuVariableNvStoreReserved to select if the
memory region described by PcdMapEmuVariableNvStoreReserved
should be mapped by the Variable Emulation driver.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
The changes can be seen at https://github.com/samimujawar/edk2/commit/a4fbf2336578546031920337239f41544a3a130e
Notes:
v1:
- Add support for mapping memory used for Non-Volatile variable [SAMI]
storage as runtime memory.
MdeModulePkg/MdeModulePkg.dec | 9 +++
MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c | 77 +++++++++++++++++++-
MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf | 6 ++
3 files changed, 90 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 6037504fa789d4ca96d45bf3a776dd77bc17a909..077d3682371e1d44b0c86f922f80536080403e52 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -8,6 +8,7 @@
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR>
# Copyright (c) 2016, Microsoft Corporation<BR>
+# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
# This program and the accompanying materials are licensed and made available under
# the terms and conditions of the BSD License that accompanies this distribution.
# The full text of the license may be found at
@@ -1592,6 +1593,14 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
# @Prompt Reserved memory range for EMU variable NV storage.
gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0|UINT64|0x40000008
+ ## Indicates if the reserved memory range for the EMU Variable driver's
+ # NV Variable Store should be mapped by the driver. The memory range size
+ # must be PcdVariableStoreSize.
+ # TRUE - Map the memory as persistent runtime memory.<BR>
+ # FALSE - Do not map the memory.<BR>
+ # @Prompt Map persistent memory range for EMU variable NV storage.
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMapEmuVariableNvStoreReserved|FALSE|BOOLEAN|0x4000000f
+
## This PCD defines the times to print hello world string.
# This PCD is a sample to explain UINT32 PCD usage.
# @Prompt HellowWorld print times.
diff --git a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
index 1bcf931b96a670ab1fc66cd66f9d88d68f363e7f..e103a340aea9394d9ff146efa5581d3180c45035 100644
--- a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
+++ b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
@@ -4,6 +4,7 @@
The nonvolatile variable space doesn't exist.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
This program and the accompanying materials
are licensed and made available under the terms and conditions of the BSD License
which accompanies this distribution. The full text of the license may be found at
@@ -14,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
+#include <Library/DxeServicesTableLib.h>
+
#include "Variable.h"
///
@@ -1643,11 +1646,13 @@ EmuQueryVariableInfo (
This function allocates memory space for variable store area and initializes its attributes.
+ @param ImageHandle The Image handle of this driver.
@param VolatileStore Indicates if the variable store is volatile.
**/
EFI_STATUS
InitializeVariableStore (
+ IN EFI_HANDLE ImageHandle,
IN BOOLEAN VolatileStore
)
{
@@ -1693,6 +1698,74 @@ InitializeVariableStore (
VariableStore =
(VARIABLE_STORE_HEADER *)(VOID*)(UINTN)
PcdGet64 (PcdEmuVariableNvStoreReserved);
+
+ if (PcdGetBool (PcdMapEmuVariableNvStoreReserved)) {
+ DEBUG((
+ DEBUG_INFO,
+ "Initializing NV Variable Store at %p, Size = 0x%x\n",
+ VariableStore, PcdGet32 (PcdVariableStoreSize)
+ ));
+
+ // Declare the NV Storage as persistent EFI_MEMORY_RUNTIME memory
+ Status = gDS->AddMemorySpace (
+ EfiGcdMemoryTypePersistent,
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize),
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR, "Failed to add memory space. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ Status = gDS->AllocateMemorySpace (
+ EfiGcdAllocateAddress,
+ EfiGcdMemoryTypePersistent,
+ 0,
+ PcdGet32 (PcdVariableStoreSize),
+ (EFI_PHYSICAL_ADDRESS*)&VariableStore,
+ ImageHandle,
+ NULL
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to allocate memory space. Status = %r\n",
+ Status
+ ));
+ gDS->RemoveMemorySpace (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize)
+ );
+ return Status;
+ }
+
+ Status = gDS->SetMemorySpaceAttributes (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize),
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to set memory attributes. Status = %r\n",
+ Status
+ ));
+ gDS->FreeMemorySpace (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize)
+ );
+ gDS->RemoveMemorySpace (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize)
+ );
+ return Status;
+ }
+ }
+
if (
(VariableStore->Size == PcdGet32 (PcdVariableStoreSize)) &&
(VariableStore->Format == VARIABLE_STORE_FORMATTED) &&
@@ -1806,7 +1879,7 @@ VariableCommonInitialize (
//
// Intialize volatile variable store
//
- Status = InitializeVariableStore (TRUE);
+ Status = InitializeVariableStore (ImageHandle, TRUE);
if (EFI_ERROR (Status)) {
FreePool(mVariableModuleGlobal);
return Status;
@@ -1814,7 +1887,7 @@ VariableCommonInitialize (
//
// Intialize non volatile variable store
//
- Status = InitializeVariableStore (FALSE);
+ Status = InitializeVariableStore (ImageHandle, FALSE);
return Status;
}
diff --git a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
index 12d52dd130e317c7ed041cf144805b8547bfc62b..1b5902749e1806143e92ccc2009a7512d57b940b 100644
--- a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
@@ -5,6 +5,7 @@
# four EFI_RUNTIME_SERVICES: SetVariable, GetVariable, GetNextVariableName and QueryVariableInfo.
#
# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
#
# This program and the accompanying materials
# are licensed and made available under the terms and conditions of the BSD License
@@ -53,6 +54,7 @@ [LibraryClasses]
BaseMemoryLib
HobLib
PcdLib
+ DxeServicesTableLib
[Protocols]
gEfiVariableArchProtocolGuid ## PRODUCES
@@ -77,10 +79,14 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMapEmuVariableNvStoreReserved ## CONSUMES
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics ## CONSUMES # statistic the information of variable.
+[Depex.common.DXE_RUNTIME_DRIVER]
+ gEfiCpuArchProtocolGuid
+
[Depex]
TRUE
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory
2018-10-12 14:40 ` [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory Sami Mujawar
@ 2018-10-13 9:09 ` Zeng, Star
2018-10-13 21:28 ` Laszlo Ersek
1 sibling, 0 replies; 21+ messages in thread
From: Zeng, Star @ 2018-10-13 9:09 UTC (permalink / raw)
To: Sami Mujawar, edk2-devel@lists.01.org
Cc: ard.biesheuvel@linaro.org, leif.lindholm@linaro.org, Wang, Jian J,
Ni, Ruiyu, evan.lloyd@arm.com, Matteo.Carlini@arm.com,
Stephanie.Hughes-Fitt@arm.com, nd@arm.com, Zeng, Star
Hi Sami,
Thanks for the patch.
I just checked this patch and roughly checked [PATCH v1 5/6] ArmVirtPkg: Add kvmtool platform driver.
I would like to suggest that the code added in this patch to be moved into KvmtoolPlatformDxe which is added by [PATCH v1 5/6] as KvmtoolPlatformDxe is the producer of KvmtoolPlatformDxe and has full knowledge to that memory usage.
Thanks,
Star
-----Original Message-----
From: Sami Mujawar [mailto:sami.mujawar@arm.com]
Sent: Friday, October 12, 2018 10:40 PM
To: edk2-devel@lists.01.org
Cc: ard.biesheuvel@linaro.org; leif.lindholm@linaro.org; Zeng, Star <star.zeng@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; evan.lloyd@arm.com; Matteo.Carlini@arm.com; Stephanie.Hughes-Fitt@arm.com; nd@arm.com
Subject: [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory
Some platforms are able to preserve a memory range across system resets. This memory can be used for the Non-Volatile variables storage. The PcdEmuVariableNvStoreReserved is used to specify this option.
This patch enables mapping of this Non-Volatile memory range as runtime memory so that the variable storage is accessible post ExitBootServices.
Also added PcdMapEmuVariableNvStoreReserved to select if the memory region described by PcdMapEmuVariableNvStoreReserved should be mapped by the Variable Emulation driver.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
The changes can be seen at https://github.com/samimujawar/edk2/commit/a4fbf2336578546031920337239f41544a3a130e
Notes:
v1:
- Add support for mapping memory used for Non-Volatile variable [SAMI]
storage as runtime memory.
MdeModulePkg/MdeModulePkg.dec | 9 +++
MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c | 77 +++++++++++++++++++-
MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf | 6 ++
3 files changed, 90 insertions(+), 2 deletions(-)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 6037504fa789d4ca96d45bf3a776dd77bc17a909..077d3682371e1d44b0c86f922f80536080403e52 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -8,6 +8,7 @@
# (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR> # Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> # Copyright (c) 2016, Microsoft Corporation<BR>
+# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
# This program and the accompanying materials are licensed and made available under # the terms and conditions of the BSD License that accompanies this distribution.
# The full text of the license may be found at @@ -1592,6 +1593,14 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
# @Prompt Reserved memory range for EMU variable NV storage.
gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0|UINT64|0x40000008
+ ## Indicates if the reserved memory range for the EMU Variable
+ driver's # NV Variable Store should be mapped by the driver. The
+ memory range size # must be PcdVariableStoreSize.
+ # TRUE - Map the memory as persistent runtime memory.<BR>
+ # FALSE - Do not map the memory.<BR>
+ # @Prompt Map persistent memory range for EMU variable NV storage.
+
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMapEmuVariableNvStoreReserved|FALSE|
+ BOOLEAN|0x4000000f
+
## This PCD defines the times to print hello world string.
# This PCD is a sample to explain UINT32 PCD usage.
# @Prompt HellowWorld print times.
diff --git a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
index 1bcf931b96a670ab1fc66cd66f9d88d68f363e7f..e103a340aea9394d9ff146efa5581d3180c45035 100644
--- a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
+++ b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
@@ -4,6 +4,7 @@
The nonvolatile variable space doesn't exist.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -14,6 +15,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
**/
+#include <Library/DxeServicesTableLib.h>
+
#include "Variable.h"
///
@@ -1643,11 +1646,13 @@ EmuQueryVariableInfo (
This function allocates memory space for variable store area and initializes its attributes.
+ @param ImageHandle The Image handle of this driver.
@param VolatileStore Indicates if the variable store is volatile.
**/
EFI_STATUS
InitializeVariableStore (
+ IN EFI_HANDLE ImageHandle,
IN BOOLEAN VolatileStore
)
{
@@ -1693,6 +1698,74 @@ InitializeVariableStore (
VariableStore =
(VARIABLE_STORE_HEADER *)(VOID*)(UINTN)
PcdGet64 (PcdEmuVariableNvStoreReserved);
+
+ if (PcdGetBool (PcdMapEmuVariableNvStoreReserved)) {
+ DEBUG((
+ DEBUG_INFO,
+ "Initializing NV Variable Store at %p, Size = 0x%x\n",
+ VariableStore, PcdGet32 (PcdVariableStoreSize)
+ ));
+
+ // Declare the NV Storage as persistent EFI_MEMORY_RUNTIME memory
+ Status = gDS->AddMemorySpace (
+ EfiGcdMemoryTypePersistent,
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize),
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR, "Failed to add memory space. Status = %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ Status = gDS->AllocateMemorySpace (
+ EfiGcdAllocateAddress,
+ EfiGcdMemoryTypePersistent,
+ 0,
+ PcdGet32 (PcdVariableStoreSize),
+ (EFI_PHYSICAL_ADDRESS*)&VariableStore,
+ ImageHandle,
+ NULL
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to allocate memory space. Status = %r\n",
+ Status
+ ));
+ gDS->RemoveMemorySpace (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize)
+ );
+ return Status;
+ }
+
+ Status = gDS->SetMemorySpaceAttributes (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize),
+ EFI_MEMORY_UC | EFI_MEMORY_RUNTIME
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to set memory attributes. Status = %r\n",
+ Status
+ ));
+ gDS->FreeMemorySpace (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize)
+ );
+ gDS->RemoveMemorySpace (
+ (EFI_PHYSICAL_ADDRESS)VariableStore,
+ PcdGet32 (PcdVariableStoreSize)
+ );
+ return Status;
+ }
+ }
+
if (
(VariableStore->Size == PcdGet32 (PcdVariableStoreSize)) &&
(VariableStore->Format == VARIABLE_STORE_FORMATTED) && @@ -1806,7 +1879,7 @@ VariableCommonInitialize (
//
// Intialize volatile variable store
//
- Status = InitializeVariableStore (TRUE);
+ Status = InitializeVariableStore (ImageHandle, TRUE);
if (EFI_ERROR (Status)) {
FreePool(mVariableModuleGlobal);
return Status;
@@ -1814,7 +1887,7 @@ VariableCommonInitialize (
//
// Intialize non volatile variable store
//
- Status = InitializeVariableStore (FALSE);
+ Status = InitializeVariableStore (ImageHandle, FALSE);
return Status;
}
diff --git a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
index 12d52dd130e317c7ed041cf144805b8547bfc62b..1b5902749e1806143e92ccc2009a7512d57b940b 100644
--- a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDx
+++ e.inf
@@ -5,6 +5,7 @@
# four EFI_RUNTIME_SERVICES: SetVariable, GetVariable, GetNextVariableName and QueryVariableInfo.
#
# Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2018, ARM Limited. All rights reserved.<BR>
#
# This program and the accompanying materials # are licensed and made available under the terms and conditions of the BSD License @@ -53,6 +54,7 @@ [LibraryClasses]
BaseMemoryLib
HobLib
PcdLib
+ DxeServicesTableLib
[Protocols]
gEfiVariableArchProtocolGuid ## PRODUCES
@@ -77,10 +79,14 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize ## CONSUMES
gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize ## CONSUMES
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMapEmuVariableNvStoreReserved ## CONSUMES
[FeaturePcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics ## CONSUMES # statistic the information of variable.
+[Depex.common.DXE_RUNTIME_DRIVER]
+ gEfiCpuArchProtocolGuid
+
[Depex]
TRUE
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory
2018-10-12 14:40 ` [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory Sami Mujawar
2018-10-13 9:09 ` Zeng, Star
@ 2018-10-13 21:28 ` Laszlo Ersek
1 sibling, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-10-13 21:28 UTC (permalink / raw)
To: Sami Mujawar, edk2-devel
Cc: ruiyu.ni, nd, Stephanie.Hughes-Fitt, star.zeng, Grant Likely
(+Grant)
On 10/12/18 16:40, Sami Mujawar wrote:
> Some platforms are able to preserve a memory range across
> system resets. This memory can be used for the Non-Volatile
> variables storage. The PcdEmuVariableNvStoreReserved is
> used to specify this option.
This is the wrong approach, for two reasons.
(1) At a technical level, suck hacks are usually implemented by provding
a platform-specific FVB (firmware volume block) protocol / driver,
underneath the normal FTW and Variable drivers. The FVB implementation
can (technically) fake a "persistent" flash device in RAM that is
actually volatile.
(2) At a design level, this is an extremely bad idea though. Such
variables are only halfway non-volatile, and that fact always comes back
to bite users, sooner or later. I speak from experience with OVMF, but
more recently, there has been discussion on the USWG list too, about
platforms that can only fake non-volatility. I can't provide more
details on this open list about that, so I'll just give you a
Message-Id, and a Mantis ticket.
<a9a12a41-a1b8-3d86-5fc1-547abcff0a35@arm.com>
https://mantis.uefi.org/mantis/view.php?id=1961
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 4/6] ArmVirtPkg: Save DT base address from X0 in PCD
2018-10-12 14:40 [PATCH 0/6] Add kvmtool emulated platform support for ARM Sami Mujawar
` (2 preceding siblings ...)
2018-10-12 14:40 ` [PATCH v1 3/6] MdeModulePkg: Map persistent (NV) memory Sami Mujawar
@ 2018-10-12 14:40 ` Sami Mujawar
2018-10-13 21:35 ` Laszlo Ersek
2018-10-12 14:40 ` [PATCH v1 5/6] ArmVirtPkg: Add kvmtool platform driver Sami Mujawar
` (2 subsequent siblings)
6 siblings, 1 reply; 21+ messages in thread
From: Sami Mujawar @ 2018-10-12 14:40 UTC (permalink / raw)
To: edk2-devel
Cc: ard.biesheuvel, leif.lindholm, lersek, julien.grall, evan.lloyd,
Matteo.Carlini, Stephanie.Hughes-Fitt, nd
Some virtual machine managers provide the base address of the DT
in memory in the X0 register. Save the DT Base address in the
PcdDeviceTreeInitialBaseAddress so that the firmware can use the
PCD to parse the DT and obtain the platform information subsequently.
This change also requires that the PcdDeviceTreeInitialBaseAddress
be a Dynamic PCD.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
The changes can be seen at https://github.com/samimujawar/edk2/commit/57ffa0da043fd73907b24a6833d2797ea3dae564
Notes:
v1:
- Enable loading of DT from base address passed in X0. [SAMI]
ArmVirtPkg/ArmVirtPkg.dec | 12 ++++++++----
ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S | 9 +++++++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index 8f656fd2739dd1460891029f53953c765396f8fb..9d4b782a43e505079263ded2347dff2304c2a75c 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -42,15 +42,19 @@ [Guids.common]
[Protocols]
gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
-[PcdsFixedAtBuild, PcdsPatchableInModule]
+[PcdsFixedAtBuild.common, PcdsDynamic.common, PcdsPatchableInModule.common]
#
# This is the physical address where the device tree is expected to be stored
- # upon first entry into UEFI. This needs to be a FixedAtBuild PCD, so that we
- # can do a first pass over the device tree in the SEC phase to discover the
- # UART base address.
+ # upon first entry into UEFI. In some cases this needs to be a FixedAtBuild
+ # PCD, so that we can do a first pass over the device tree in the SEC phase
+ # to discover the UART base address. In other cases where the base address
+ # for the DT is passed in X0 (by an earlier firmware stage or by a virtual
+ # machine manager), this needs to be a PcdsDynamic so that the value can be
+ # updated.
#
gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x0|UINT64|0x00000001
+[PcdsFixedAtBuild]
#
# Padding in bytes to add to the device tree allocation, so that the DTB can
# be modified in place (default: 256 bytes)
diff --git a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S
index 891cf1fcab400c0842035be6a2fb003bafd63040..b5f8c9dd3c6a2bd2c937ff3891b2f51be65094d4 100644
--- a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S
+++ b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S
@@ -1,5 +1,5 @@
//
-// Copyright (c) 2011-2013, ARM Limited. All rights reserved.
+// Copyright (c) 2011-2018, ARM Limited. All rights reserved.
// Copyright (c) 2015-2016, Linaro Limited. All rights reserved.
//
// This program and the accompanying materials
@@ -143,8 +143,13 @@ ASM_PFX(DiscoverDramFromDt):
//
cbnz x0, 0f
ldr x0, PcdGet64 (PcdDeviceTreeInitialBaseAddress)
+ b 1f
-0:mov x29, x30 // preserve LR
+ // Store the device tree base address.
+0:adr x8, PcdGet64 (PcdDeviceTreeInitialBaseAddress)
+ str x0, [x8]
+
+1:mov x29, x30 // preserve LR
mov x28, x0 // preserve DTB pointer
mov x27, x1 // preserve base of image pointer
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 4/6] ArmVirtPkg: Save DT base address from X0 in PCD
2018-10-12 14:40 ` [PATCH v1 4/6] ArmVirtPkg: Save DT base address from X0 in PCD Sami Mujawar
@ 2018-10-13 21:35 ` Laszlo Ersek
2018-10-19 14:01 ` Ard Biesheuvel
0 siblings, 1 reply; 21+ messages in thread
From: Laszlo Ersek @ 2018-10-13 21:35 UTC (permalink / raw)
To: Sami Mujawar, edk2-devel
Cc: nd, Stephanie.Hughes-Fitt, Drew Jones, Eric Auger, Ard Biesheuvel
On 10/12/18 16:40, Sami Mujawar wrote:
> Some virtual machine managers provide the base address of the DT
> in memory in the X0 register. Save the DT Base address in the
> PcdDeviceTreeInitialBaseAddress so that the firmware can use the
> PCD to parse the DT and obtain the platform information subsequently.
>
> This change also requires that the PcdDeviceTreeInitialBaseAddress
> be a Dynamic PCD.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> The changes can be seen at https://github.com/samimujawar/edk2/commit/57ffa0da043fd73907b24a6833d2797ea3dae564
>
> Notes:
> v1:
> - Enable loading of DT from base address passed in X0. [SAMI]
>
> ArmVirtPkg/ArmVirtPkg.dec | 12 ++++++++----
> ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S | 9 +++++++--
> 2 files changed, 15 insertions(+), 6 deletions(-)
This patch seems to overlap with a feature that (I think) Ard, Drew and
Eric have discussed earlier, for QEMU (floating RAM base). I suggest
coordinating as early as possible.
Thanks
Laszlo
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index 8f656fd2739dd1460891029f53953c765396f8fb..9d4b782a43e505079263ded2347dff2304c2a75c 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -42,15 +42,19 @@ [Guids.common]
> [Protocols]
> gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
>
> -[PcdsFixedAtBuild, PcdsPatchableInModule]
> +[PcdsFixedAtBuild.common, PcdsDynamic.common, PcdsPatchableInModule.common]
> #
> # This is the physical address where the device tree is expected to be stored
> - # upon first entry into UEFI. This needs to be a FixedAtBuild PCD, so that we
> - # can do a first pass over the device tree in the SEC phase to discover the
> - # UART base address.
> + # upon first entry into UEFI. In some cases this needs to be a FixedAtBuild
> + # PCD, so that we can do a first pass over the device tree in the SEC phase
> + # to discover the UART base address. In other cases where the base address
> + # for the DT is passed in X0 (by an earlier firmware stage or by a virtual
> + # machine manager), this needs to be a PcdsDynamic so that the value can be
> + # updated.
> #
> gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x0|UINT64|0x00000001
>
> +[PcdsFixedAtBuild]
> #
> # Padding in bytes to add to the device tree allocation, so that the DTB can
> # be modified in place (default: 256 bytes)
> diff --git a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S
> index 891cf1fcab400c0842035be6a2fb003bafd63040..b5f8c9dd3c6a2bd2c937ff3891b2f51be65094d4 100644
> --- a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S
> +++ b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S
> @@ -1,5 +1,5 @@
> //
> -// Copyright (c) 2011-2013, ARM Limited. All rights reserved.
> +// Copyright (c) 2011-2018, ARM Limited. All rights reserved.
> // Copyright (c) 2015-2016, Linaro Limited. All rights reserved.
> //
> // This program and the accompanying materials
> @@ -143,8 +143,13 @@ ASM_PFX(DiscoverDramFromDt):
> //
> cbnz x0, 0f
> ldr x0, PcdGet64 (PcdDeviceTreeInitialBaseAddress)
> + b 1f
>
> -0:mov x29, x30 // preserve LR
> + // Store the device tree base address.
> +0:adr x8, PcdGet64 (PcdDeviceTreeInitialBaseAddress)
> + str x0, [x8]
> +
> +1:mov x29, x30 // preserve LR
> mov x28, x0 // preserve DTB pointer
> mov x27, x1 // preserve base of image pointer
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v1 4/6] ArmVirtPkg: Save DT base address from X0 in PCD
2018-10-13 21:35 ` Laszlo Ersek
@ 2018-10-19 14:01 ` Ard Biesheuvel
0 siblings, 0 replies; 21+ messages in thread
From: Ard Biesheuvel @ 2018-10-19 14:01 UTC (permalink / raw)
To: Laszlo Ersek
Cc: Sami Mujawar, edk2-devel@lists.01.org, nd, Stephanie Hughes-Fitt,
Drew Jones, Eric Auger
On 14 October 2018 at 05:35, Laszlo Ersek <lersek@redhat.com> wrote:
> On 10/12/18 16:40, Sami Mujawar wrote:
>> Some virtual machine managers provide the base address of the DT
>> in memory in the X0 register. Save the DT Base address in the
>> PcdDeviceTreeInitialBaseAddress so that the firmware can use the
>> PCD to parse the DT and obtain the platform information subsequently.
>>
>> This change also requires that the PcdDeviceTreeInitialBaseAddress
>> be a Dynamic PCD.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
>> ---
>> The changes can be seen at https://github.com/samimujawar/edk2/commit/57ffa0da043fd73907b24a6833d2797ea3dae564
>>
>> Notes:
>> v1:
>> - Enable loading of DT from base address passed in X0. [SAMI]
>>
>> ArmVirtPkg/ArmVirtPkg.dec | 12 ++++++++----
>> ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S | 9 +++++++--
>> 2 files changed, 15 insertions(+), 6 deletions(-)
>
> This patch seems to overlap with a feature that (I think) Ard, Drew and
> Eric have discussed earlier, for QEMU (floating RAM base). I suggest
> coordinating as early as possible.
>
This is the PrePi code, which we only use currently for
ArmVirtQemuKernel and ArmVirtXen.
PrePi is a hack which I am only willing to tolerate for legacy
platforms, and for platforms that require relocation, i.e., where the
runtime address of the SEC module is not known at link time.
I have asked Sami [in person] to look into whether we can avoid using
this PCD entirely, or at least avoid switching to a dynamic PCD.
Instead, I would prefer an approach where we store the value of X0 in
a global variable (which is permitted given that a self relocating
PrePi can only execute from DRAM anyway), and create the FDT GUID HOB
directly once we are running C code.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 5/6] ArmVirtPkg: Add kvmtool platform driver
2018-10-12 14:40 [PATCH 0/6] Add kvmtool emulated platform support for ARM Sami Mujawar
` (3 preceding siblings ...)
2018-10-12 14:40 ` [PATCH v1 4/6] ArmVirtPkg: Save DT base address from X0 in PCD Sami Mujawar
@ 2018-10-12 14:40 ` Sami Mujawar
2018-10-13 21:54 ` Laszlo Ersek
2018-10-12 14:40 ` [PATCH v1 6/6] ArmVirtPkg: Support for kvmtool emulated platform Sami Mujawar
2018-10-13 21:42 ` [PATCH 0/6] Add kvmtool emulated platform support for ARM Laszlo Ersek
6 siblings, 1 reply; 21+ messages in thread
From: Sami Mujawar @ 2018-10-12 14:40 UTC (permalink / raw)
To: edk2-devel
Cc: ard.biesheuvel, leif.lindholm, lersek, julien.grall, evan.lloyd,
Matteo.Carlini, Stephanie.Hughes-Fitt, nd
The KvmtoolPlatformDxe performs the platform specific
initialization like:
- It parses the kvmtool DT for Non-Volatile memory
range to use for runtime variable storage and
initialises the PcdEmuVariableNvStoreReserved.
- It decides if the firmware should expose ACPI or
Device Tree-based hardware description to the
operating system.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
The changes can be seen at https://github.com/samimujawar/edk2/commit/57ffa0da043fd73907b24a6833d2797ea3dae564
Notes:
v1:
- Add kvmtool platform driver to support loading platform [SAMI]
specific information.
ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c | 211 ++++++++++++++++++++
ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf | 60 ++++++
2 files changed, 271 insertions(+)
diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
new file mode 100644
index 0000000000000000000000000000000000000000..c8c1d50882bb9205194214217d17ed5f3644cc98
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
@@ -0,0 +1,211 @@
+/** @file
+
+ The KvmtoolPlatformDxe performs the platform specific initialization like:
+ - It parses the kvmtool DT for Non-Volatile memory range to use for runtime
+ variable storage and initialises the PcdEmuVariableNvStoreReserved.
+ - It decides if the firmware should expose ACPI or Device Tree-based
+ hardware description to the operating system.
+
+ Copyright (c) 2018, ARM Limited. All rights reserved.
+
+ This program and the accompanying materials are licensed and made available
+ under the terms and conditions of the BSD License which accompanies this
+ distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Protocol/FdtClient.h>
+
+/** Parse the kvmtool DT for Non-Volatile Memory range and initialize
+ PcdEmuVariableNvStoreReserved.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_ACCESS_DENIED Failed to update Pcd.
+ @retval EFI_BUFFER_TOO_SMALL Non-Volatile memory region less than storage
+ required for runtime variable storage.
+**/
+STATIC
+EFI_STATUS
+InitializeNvStorageBase (
+ VOID
+)
+{
+ EFI_STATUS Status;
+ FDT_CLIENT_PROTOCOL * FdtClient;
+ INT32 Node;
+ CONST UINT64 * Reg;
+ UINT32 Len;
+ UINT64 RegSize;
+ UINT64 RegBase;
+ RETURN_STATUS PcdStatus;
+
+ Status = gBS->LocateProtocol (
+ &gFdtClientProtocolGuid,
+ NULL,
+ (VOID **)&FdtClient
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to locate Fdt Client Protocol. Status = %r\n",
+ Status
+ ));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
+
+ Status = FdtClient->FindNextCompatibleNode (
+ FdtClient,
+ "kvmtool,NVMem",
+ Node,
+ &Node
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Cannot find NV memory DT node to use for Runtime variable storage."
+ " Expected node in DT is \'compatible = \"kvmtool,NVMem\"\'."
+ " Status = %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
+
+ Status = FdtClient->GetNodeProperty (
+ FdtClient,
+ Node,
+ "reg",
+ (CONST VOID **)&Reg,
+ &Len
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: GetNodeProperty () failed. Status = %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ if (Len != (2 * sizeof (UINT64))) {
+ Status = EFI_INVALID_PARAMETER;
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Invalid DT Node data. Status = %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ RegBase = SwapBytes64 (((CONST UINT64 *)Reg)[0]);
+ RegSize = SwapBytes64 (((CONST UINT64 *)Reg)[1]);
+ DEBUG ((DEBUG_INFO, "RegBase = 0x%lx, RegSize = 0x%lx\n", RegBase, RegSize));
+
+ if (RegSize < PcdGet32 (PcdVariableStoreSize)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Not enough NV memory available for Runtime variable storage\n"
+ ));
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, RegBase);
+ if (RETURN_ERROR (PcdStatus)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to update PcdEmuVariableNvStoreReserved. Status = %r\n",
+ PcdStatus
+ ));
+ ASSERT_RETURN_ERROR (PcdStatus);
+ return EFI_ACCESS_DENIED;
+ }
+ return EFI_SUCCESS;
+}
+
+/** Decide if the firmware should expose ACPI tables or Device Tree and
+ install the appropriate protocol interface.
+
+ @param [in] ImageHandle Handle for this image.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_OUT_OF_RESOURCES There was not enough memory to install the
+ protocols.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+
+**/
+STATIC
+EFI_STATUS
+PlatformHasAcpiDt (
+ IN EFI_HANDLE ImageHandle
+ )
+{
+ if (!PcdGetBool (PcdForceNoAcpi)) {
+ // Expose ACPI tables
+ return gBS->InstallProtocolInterface (
+ &ImageHandle,
+ &gEdkiiPlatformHasAcpiGuid,
+ EFI_NATIVE_INTERFACE,
+ NULL
+ );
+ }
+
+ // Expose the Device Tree.
+ return gBS->InstallProtocolInterface (
+ &ImageHandle,
+ &gEdkiiPlatformHasDeviceTreeGuid,
+ EFI_NATIVE_INTERFACE,
+ NULL
+ );
+}
+
+/** Entry point for Kvmtool Platform Dxe
+
+ @param [in] ImageHandle Handle for this image.
+ @param [in] SystemTable Pointer to the EFI system table.
+
+ @retval EFI_SUCCESS Success.
+ @retval EFI_OUT_OF_RESOURCES There was not enough memory to install the
+ protocols.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+
+**/
+EFI_STATUS
+EFIAPI
+KvmtoolPlatformDxeEntryPoint (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = InitializeNvStorageBase ();
+ if (EFI_ERROR (Status)) {
+ goto Failed;
+ }
+
+ Status = PlatformHasAcpiDt (ImageHandle);
+ if (EFI_ERROR (Status)) {
+ goto Failed;
+ }
+
+ return Status;
+
+Failed:
+ ASSERT_EFI_ERROR (Status);
+ CpuDeadLoop ();
+
+ return Status;
+}
diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
new file mode 100644
index 0000000000000000000000000000000000000000..dddacacdc8e6182e0d063d32e3ae7ff8432c4b20
--- /dev/null
+++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
@@ -0,0 +1,60 @@
+#/** @file
+#
+# The KvmtoolPlatformDxe performs the platform specific initialization like:
+# - It parses the kvmtool DT for Non-Volatile memory range to use for runtime
+# variable storage and initialises the PcdEmuVariableNvStoreReserved.
+# - It decides if the firmware should expose ACPI or Device Tree-based
+# hardware description to the operating system.
+#
+# Copyright (c) 2018, ARM Limited. All rights reserved.
+#
+# This program and the accompanying materials are licensed and made available
+# under the terms and conditions of the BSD License which accompanies this
+# distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
+# IMPLIED.
+#
+#**/
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = KvmtoolPlatformDxe
+ FILE_GUID = 7479CCCD-D721-442A-8C73-A72DBB886669
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = KvmtoolPlatformDxeEntryPoint
+
+[Sources]
+ KvmtoolPlatformDxe.c
+
+[Packages]
+ ArmVirtPkg/ArmVirtPkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
+ MemoryAllocationLib
+ UefiBootServicesTableLib
+ UefiDriverEntryPoint
+
+[Guids]
+ gEdkiiPlatformHasAcpiGuid ## SOMETIMES_PRODUCES ## PROTOCOL
+ gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
+
+[Pcd]
+ gArmVirtTokenSpaceGuid.PcdForceNoAcpi
+ gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
+ gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize
+
+[Protocols]
+ gFdtClientProtocolGuid ## CONSUMES
+
+[Depex]
+ gFdtClientProtocolGuid
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 5/6] ArmVirtPkg: Add kvmtool platform driver
2018-10-12 14:40 ` [PATCH v1 5/6] ArmVirtPkg: Add kvmtool platform driver Sami Mujawar
@ 2018-10-13 21:54 ` Laszlo Ersek
0 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-10-13 21:54 UTC (permalink / raw)
To: Sami Mujawar, edk2-devel; +Cc: nd, Stephanie.Hughes-Fitt
On 10/12/18 16:40, Sami Mujawar wrote:
> The KvmtoolPlatformDxe performs the platform specific
> initialization like:
> - It parses the kvmtool DT for Non-Volatile memory
> range to use for runtime variable storage and
> initialises the PcdEmuVariableNvStoreReserved.
> - It decides if the firmware should expose ACPI or
> Device Tree-based hardware description to the
> operating system.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> The changes can be seen at https://github.com/samimujawar/edk2/commit/57ffa0da043fd73907b24a6833d2797ea3dae564
>
> Notes:
> v1:
> - Add kvmtool platform driver to support loading platform [SAMI]
> specific information.
>
> ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c | 211 ++++++++++++++++++++
> ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf | 60 ++++++
> 2 files changed, 271 insertions(+)
>
> diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..c8c1d50882bb9205194214217d17ed5f3644cc98
> --- /dev/null
> +++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
> @@ -0,0 +1,211 @@
> +/** @file
> +
> + The KvmtoolPlatformDxe performs the platform specific initialization like:
> + - It parses the kvmtool DT for Non-Volatile memory range to use for runtime
> + variable storage and initialises the PcdEmuVariableNvStoreReserved.
> + - It decides if the firmware should expose ACPI or Device Tree-based
> + hardware description to the operating system.
> +
> + Copyright (c) 2018, ARM Limited. All rights reserved.
> +
> + This program and the accompanying materials are licensed and made available
> + under the terms and conditions of the BSD License which accompanies this
> + distribution. The full text of the license may be found at
> + http://opensource.org/licenses/bsd-license.php
> +
> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiDriverEntryPoint.h>
> +#include <Protocol/FdtClient.h>
> +
> +/** Parse the kvmtool DT for Non-Volatile Memory range and initialize
> + PcdEmuVariableNvStoreReserved.
> +
> + @retval EFI_SUCCESS Success.
> + @retval EFI_ACCESS_DENIED Failed to update Pcd.
> + @retval EFI_BUFFER_TOO_SMALL Non-Volatile memory region less than storage
> + required for runtime variable storage.
> +**/
> +STATIC
> +EFI_STATUS
> +InitializeNvStorageBase (
> + VOID
> +)
> +{
> + EFI_STATUS Status;
> + FDT_CLIENT_PROTOCOL * FdtClient;
> + INT32 Node;
> + CONST UINT64 * Reg;
> + UINT32 Len;
> + UINT64 RegSize;
> + UINT64 RegBase;
> + RETURN_STATUS PcdStatus;
> +
> + Status = gBS->LocateProtocol (
> + &gFdtClientProtocolGuid,
> + NULL,
> + (VOID **)&FdtClient
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "Failed to locate Fdt Client Protocol. Status = %r\n",
> + Status
> + ));
> + ASSERT_EFI_ERROR (Status);
> + return Status;
> + }
> +
> + Status = FdtClient->FindNextCompatibleNode (
> + FdtClient,
> + "kvmtool,NVMem",
> + Node,
> + &Node
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Cannot find NV memory DT node to use for Runtime variable storage."
> + " Expected node in DT is \'compatible = \"kvmtool,NVMem\"\'."
> + " Status = %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + ASSERT_EFI_ERROR (Status);
> + return Status;
> + }
> +
> + Status = FdtClient->GetNodeProperty (
> + FdtClient,
> + Node,
> + "reg",
> + (CONST VOID **)&Reg,
> + &Len
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: GetNodeProperty () failed. Status = %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> +
> + if (Len != (2 * sizeof (UINT64))) {
> + Status = EFI_INVALID_PARAMETER;
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Invalid DT Node data. Status = %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> +
> + RegBase = SwapBytes64 (((CONST UINT64 *)Reg)[0]);
> + RegSize = SwapBytes64 (((CONST UINT64 *)Reg)[1]);
> + DEBUG ((DEBUG_INFO, "RegBase = 0x%lx, RegSize = 0x%lx\n", RegBase, RegSize));
> +
> + if (RegSize < PcdGet32 (PcdVariableStoreSize)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "Not enough NV memory available for Runtime variable storage\n"
> + ));
> + return EFI_BUFFER_TOO_SMALL;
> + }
> +
> + PcdStatus = PcdSet64S (PcdEmuVariableNvStoreReserved, RegBase);
> + if (RETURN_ERROR (PcdStatus)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "Failed to update PcdEmuVariableNvStoreReserved. Status = %r\n",
> + PcdStatus
> + ));
> + ASSERT_RETURN_ERROR (PcdStatus);
> + return EFI_ACCESS_DENIED;
> + }
> + return EFI_SUCCESS;
> +}
It's likely best to keep this code in the platform-specific FVB driver
itself (see my comments on v1 3/6).
With the main point remaining, of course, that almost-non-volatile
variables are a very bad idea.
> +/** Decide if the firmware should expose ACPI tables or Device Tree and
> + install the appropriate protocol interface.
> +
> + @param [in] ImageHandle Handle for this image.
> +
> + @retval EFI_SUCCESS Success.
> + @retval EFI_OUT_OF_RESOURCES There was not enough memory to install the
> + protocols.
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +
> +**/
> +STATIC
> +EFI_STATUS
> +PlatformHasAcpiDt (
> + IN EFI_HANDLE ImageHandle
> + )
> +{
> + if (!PcdGetBool (PcdForceNoAcpi)) {
> + // Expose ACPI tables
> + return gBS->InstallProtocolInterface (
> + &ImageHandle,
> + &gEdkiiPlatformHasAcpiGuid,
> + EFI_NATIVE_INTERFACE,
> + NULL
> + );
> + }
> +
> + // Expose the Device Tree.
> + return gBS->InstallProtocolInterface (
> + &ImageHandle,
> + &gEdkiiPlatformHasDeviceTreeGuid,
> + EFI_NATIVE_INTERFACE,
> + NULL
> + );
> +}
This function seems to be derived from
"ArmVirtPkg/PlatformHasAcpiDtDxe", by dropping the word size check, and
the fw_cfg check. Is that correct? I think it's worth documenting.
(Especially the fact that, on this platform, PcdForceNoAcpi will be a
plain dynamic PCD, not an HII one; which is why the INF file need not
depend on the variable architectural protocol.)
Thanks
Laszlo
> +
> +/** Entry point for Kvmtool Platform Dxe
> +
> + @param [in] ImageHandle Handle for this image.
> + @param [in] SystemTable Pointer to the EFI system table.
> +
> + @retval EFI_SUCCESS Success.
> + @retval EFI_OUT_OF_RESOURCES There was not enough memory to install the
> + protocols.
> + @retval EFI_INVALID_PARAMETER A parameter is invalid.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +KvmtoolPlatformDxeEntryPoint (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = InitializeNvStorageBase ();
> + if (EFI_ERROR (Status)) {
> + goto Failed;
> + }
> +
> + Status = PlatformHasAcpiDt (ImageHandle);
> + if (EFI_ERROR (Status)) {
> + goto Failed;
> + }
> +
> + return Status;
> +
> +Failed:
> + ASSERT_EFI_ERROR (Status);
> + CpuDeadLoop ();
> +
> + return Status;
> +}
> diff --git a/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
> new file mode 100644
> index 0000000000000000000000000000000000000000..dddacacdc8e6182e0d063d32e3ae7ff8432c4b20
> --- /dev/null
> +++ b/ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
> @@ -0,0 +1,60 @@
> +#/** @file
> +#
> +# The KvmtoolPlatformDxe performs the platform specific initialization like:
> +# - It parses the kvmtool DT for Non-Volatile memory range to use for runtime
> +# variable storage and initialises the PcdEmuVariableNvStoreReserved.
> +# - It decides if the firmware should expose ACPI or Device Tree-based
> +# hardware description to the operating system.
> +#
> +# Copyright (c) 2018, ARM Limited. All rights reserved.
> +#
> +# This program and the accompanying materials are licensed and made available
> +# under the terms and conditions of the BSD License which accompanies this
> +# distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +#
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> +# IMPLIED.
> +#
> +#**/
> +
> +[Defines]
> + INF_VERSION = 0x0001001B
> + BASE_NAME = KvmtoolPlatformDxe
> + FILE_GUID = 7479CCCD-D721-442A-8C73-A72DBB886669
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + ENTRY_POINT = KvmtoolPlatformDxeEntryPoint
> +
> +[Sources]
> + KvmtoolPlatformDxe.c
> +
> +[Packages]
> + ArmVirtPkg/ArmVirtPkg.dec
> + EmbeddedPkg/EmbeddedPkg.dec
> + MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> + BaseMemoryLib
> + DebugLib
> + MemoryAllocationLib
> + UefiBootServicesTableLib
> + UefiDriverEntryPoint
> +
> +[Guids]
> + gEdkiiPlatformHasAcpiGuid ## SOMETIMES_PRODUCES ## PROTOCOL
> + gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
> +
> +[Pcd]
> + gArmVirtTokenSpaceGuid.PcdForceNoAcpi
> + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
> + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize
> +
> +[Protocols]
> + gFdtClientProtocolGuid ## CONSUMES
> +
> +[Depex]
> + gFdtClientProtocolGuid
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v1 6/6] ArmVirtPkg: Support for kvmtool emulated platform
2018-10-12 14:40 [PATCH 0/6] Add kvmtool emulated platform support for ARM Sami Mujawar
` (4 preceding siblings ...)
2018-10-12 14:40 ` [PATCH v1 5/6] ArmVirtPkg: Add kvmtool platform driver Sami Mujawar
@ 2018-10-12 14:40 ` Sami Mujawar
2018-10-13 21:57 ` Laszlo Ersek
2018-10-13 21:42 ` [PATCH 0/6] Add kvmtool emulated platform support for ARM Laszlo Ersek
6 siblings, 1 reply; 21+ messages in thread
From: Sami Mujawar @ 2018-10-12 14:40 UTC (permalink / raw)
To: edk2-devel
Cc: ard.biesheuvel, leif.lindholm, lersek, julien.grall, evan.lloyd,
Matteo.Carlini, Stephanie.Hughes-Fitt, nd
Kvmtool is a virtual machine manager that enables hosting
KVM guests. Kvmtool emulates certain devices like serial
port, RTC, etc. essentially providing an emulated platform.
This patch adds support for kvmtool emulated platform.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---
The changes can be seen at https://github.com/samimujawar/edk2/commit/f13ce7024f7216e4036459bab6ce4b1d700f6050
Notes:
v1:
- Add support for Kvmtool emulated platform [SAMI]
ArmVirtPkg/ArmVirtKvmTool.dsc | 390 ++++++++++++++++++++
ArmVirtPkg/ArmVirtKvmTool.fdf | 297 +++++++++++++++
2 files changed, 687 insertions(+)
diff --git a/ArmVirtPkg/ArmVirtKvmTool.dsc b/ArmVirtPkg/ArmVirtKvmTool.dsc
new file mode 100644
index 0000000000000000000000000000000000000000..fd3fbd32e260dc5949f129206d7519d006c3222a
--- /dev/null
+++ b/ArmVirtPkg/ArmVirtKvmTool.dsc
@@ -0,0 +1,390 @@
+# @file
+# Workspace file for KVMTool virtual platform.
+#
+# Copyright (c) 2018, ARM Limited. All rights reserved.
+#
+# This program and the accompanying materials
+# are licensed and made available under the terms and conditions of the BSD License
+# which accompanies this distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+#
+
+################################################################################
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+################################################################################
+[Defines]
+ PLATFORM_NAME = ArmVirtKvmTool
+ PLATFORM_GUID = 4CB2C61E-FA32-4130-8E37-54ABC71A1A43
+ PLATFORM_VERSION = 0.1
+ DSC_SPECIFICATION = 0x0001001B
+ OUTPUT_DIRECTORY = Build/ArmVirtKvmTool-$(ARCH)
+ SUPPORTED_ARCHITECTURES = AARCH64|ARM
+ BUILD_TARGETS = DEBUG|RELEASE|NOOPT
+ SKUID_IDENTIFIER = DEFAULT
+ FLASH_DEFINITION = ArmVirtPkg/ArmVirtKvmTool.fdf
+
+ #
+ # Defines for default states. These can be changed on the command line.
+ # -D FLAG=VALUE
+ #
+ DEFINE SECURE_BOOT_ENABLE = FALSE
+ DEFINE HTTP_BOOT_ENABLE = FALSE
+ DEFINE TTY_TERMINAL = TRUE
+ DEFINE ENABLE_NETWORK = TRUE
+
+!include ArmVirtPkg/ArmVirt.dsc.inc
+
+[LibraryClasses.common]
+ ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
+ ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+
+ # Virtio Support
+ VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf
+ VirtioMmioDeviceLib|OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
+
+ ArmPlatformLib|ArmPlatformPkg/Library/ArmPlatformLibNull/ArmPlatformLibNull.inf
+ ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
+
+ TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
+
+ CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
+ BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
+ PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+ CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
+ FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
+
+ FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
+ PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+ PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
+ PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
+
+!if $(HTTP_BOOT_ENABLE) == TRUE
+ HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
+!endif
+
+[LibraryClasses.common, LibraryClasses.common.SEC, LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
+ SerialPortLib|PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf
+ PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
+
+[LibraryClasses.common.UEFI_DRIVER]
+ UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
+
+[BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE]
+ # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE
+ # executable we build for the relocatable PrePi. They are not runtime
+ # relocatable in ELF.
+ *_CLANG35_*_CC_FLAGS = -mno-movt
+
+################################################################################
+#
+# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+#
+################################################################################
+
+[PcdsFeatureFlag.common]
+ ## If TRUE, Graphics Output Protocol will be installed on virtual handle created by ConsplitterDxe.
+ # It could be set FALSE to save size.
+ gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
+ gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
+
+[PcdsFixedAtBuild.common]
+ gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F
+
+ gArmPlatformTokenSpaceGuid.PcdCoreCount|1
+!if $(ARCH) == AARCH64
+ gArmTokenSpaceGuid.PcdVFPEnabled|1
+!endif
+
+ gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x4007c000
+ gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
+
+ # Size of the region used by UEFI in permanent memory (Reserved 64MB)
+ gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x04000000
+
+ ## Trustzone enable (to make the transition from EL3 to EL2 in ArmPlatformPkg/Sec)
+ gArmTokenSpaceGuid.PcdTrustzoneSupport|FALSE
+
+ #
+ # ARM PrimeCell
+ #
+ ## Default Terminal Type
+ ## 0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-TTYTERM
+#!if $(TTY_TERMINAL) == TRUE
+ gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
+#!else
+# gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1
+#!endif
+
+ #
+ # ARM Virtual Architectural Timer -- fetch frequency from QEMU (TCG) or KVM
+ #
+ gArmTokenSpaceGuid.PcdArmArchTimerFreqInHz|0
+
+!if $(HTTP_BOOT_ENABLE) == TRUE
+ gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
+!endif
+
+ # Use MMIO for accessing Serial port registers.
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
+
+ # Use MMIO for accessing RTC controller registers.
+ gPcAtChipsetPkgTokenSpaceGuid.PcdRtcUseMmio|TRUE
+
+ # Enable mapping of Emulated Variable storage post Exit Boot Services
+ gEfiMdeModulePkgTokenSpaceGuid.PcdMapEmuVariableNvStoreReserved|TRUE
+
+[PcdsPatchableInModule.common]
+ #
+ # This will be overridden in the code
+ #
+ gArmTokenSpaceGuid.PcdSystemMemoryBase|0x0
+ gArmTokenSpaceGuid.PcdSystemMemorySize|0x0
+
+ #
+ # Define a default initial address for the device tree.
+ # Ignored if x0 != 0 at entry.
+ #
+ gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x0
+
+ gArmTokenSpaceGuid.PcdFdBaseAddress|0x0
+ gArmTokenSpaceGuid.PcdFvBaseAddress|0x0
+
+[PcdsFixedAtBuild.AARCH64]
+ gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
+ gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
+
+ #
+ # The maximum physical I/O addressability of the processor, set with
+ # BuildCpuHob().
+ #
+ gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
+
+ # KVM limits it IPA space to 40 bits (1 TB), so there is no need to
+ # support anything bigger, even if the host hardware does
+ gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40
+
+[PcdsDynamicDefault.common]
+ gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
+
+ gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum|0x0
+ gArmTokenSpaceGuid.PcdArmArchTimerIntrNum|0x0
+ gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum|0x0
+ gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum|0x0
+
+ #
+ # ARM General Interrupt Controller
+ #
+ gArmTokenSpaceGuid.PcdGicDistributorBase|0x0
+ gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x0
+ gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0x0
+
+ #
+ # PCI settings
+ #
+ gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|TRUE
+
+ # set PcdPciExpressBaseAddress to MAX_UINT64, which signifies that this
+ # PCD and PcdPciDisableBusEnumeration above have not been assigned yet
+ gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xFFFFFFFFFFFFFFFF
+
+ gArmTokenSpaceGuid.PcdPciIoTranslation|0x0
+
+ #
+ # Set video resolution for boot options and for text setup.
+ # PlatformDxe can set the former at runtime.
+ #
+ gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|800
+ gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|600
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution|640
+ gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution|480
+
+ ## Force DTB
+ gArmVirtTokenSpaceGuid.PcdForceNoAcpi|TRUE
+
+ ## Emulate Runtime variable in kvmtool provided NV memory. The
+ # EmuVariableStorageFdtDxe will parse the kvmtool DT and update
+ # the PcdEmuVariableNvStoreReserved at runtime.
+ gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0
+
+################################################################################
+#
+# Components Section - list of all EDK II Modules needed by this Platform
+#
+################################################################################
+[Components.common]
+ #
+ # PEI Phase modules
+ #
+ ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf {
+ <LibraryClasses>
+ ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/PrePiExtractGuidedSectionLib.inf
+ LzmaDecompressLib|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
+ PrePiLib|EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
+ HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
+ PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
+ MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
+ }
+
+ #
+ # DXE
+ #
+ MdeModulePkg/Core/Dxe/DxeMain.inf {
+ <LibraryClasses>
+ NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
+ DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
+ }
+ MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
+ <LibraryClasses>
+ PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+ }
+
+ #
+ # Architectural Protocols
+ #
+ ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+ MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
+ MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf {
+ <LibraryClasses>
+ # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
+ BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+ }
+
+!if $(SECURE_BOOT_ENABLE) == TRUE
+ MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
+ <LibraryClasses>
+ NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+ }
+ SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+!else
+ MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
+!endif
+ MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
+ MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
+ MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
+ MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
+ MdeModulePkg/Universal/Metronome/Metronome.inf
+ PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
+
+ MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
+ MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
+ MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
+ MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
+ MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
+
+ MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
+
+ ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+ ArmPkg/Drivers/TimerDxe/TimerDxe.inf {
+ <LibraryClasses>
+ NULL|ArmVirtPkg/Library/ArmVirtTimerFdtClientLib/ArmVirtTimerFdtClientLib.inf
+ }
+ MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
+
+ #
+ # Platform Driver
+ #
+ ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
+ ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
+ ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+ ArmVirtPkg/HighMemDxe/HighMemDxe.inf
+ OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
+ OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
+ OvmfPkg/VirtioNetDxe/VirtioNet.inf
+ OvmfPkg/VirtioRngDxe/VirtioRng.inf
+
+ #
+ # FAT filesystem + GPT/MBR partitioning + UDF filesystem
+ #
+ MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
+ MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
+ MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
+ FatPkg/EnhancedFatDxe/Fat.inf
+ MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
+
+ #
+ # Bds
+ #
+ MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf {
+ <LibraryClasses>
+ DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
+ PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+ }
+ MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
+ MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+ MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
+ MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+ MdeModulePkg/Logo/LogoDxe.inf
+ MdeModulePkg/Application/UiApp/UiApp.inf {
+ <LibraryClasses>
+ NULL|MdeModulePkg/Library/DeviceManagerUiLib/DeviceManagerUiLib.inf
+ NULL|MdeModulePkg/Library/BootManagerUiLib/BootManagerUiLib.inf
+ NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
+ }
+
+!if $(ENABLE_NETWORK) == TRUE
+ #
+ # Networking stack
+ #
+ MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
+ MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
+ MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
+ MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
+ MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
+ MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
+ MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
+ MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
+ MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
+ MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
+ MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
+ MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
+!if $(HTTP_BOOT_ENABLE) == TRUE
+ NetworkPkg/DnsDxe/DnsDxe.inf
+ NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
+ NetworkPkg/HttpDxe/HttpDxe.inf
+ NetworkPkg/HttpBootDxe/HttpBootDxe.inf
+!endif
+!endif
+ #
+ # SCSI Bus and Disk Driver
+ #
+ MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
+ MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
+
+ #
+ # PCI support
+ #
+ ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf {
+ <LibraryClasses>
+ NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+ }
+ MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+ MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
+ <LibraryClasses>
+ NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+ }
+ OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
+ OvmfPkg/Virtio10Dxe/Virtio10.inf
+
+ #
+ # Video support
+ #
+ OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
+ OvmfPkg/PlatformDxe/Platform.inf
+
+ #
+ # USB Support
+ #
+ MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
+ MdeModulePkg/Bus/Pci/EhciDxe/EhciDxe.inf
+ MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
+ MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
+ MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
+ MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
diff --git a/ArmVirtPkg/ArmVirtKvmTool.fdf b/ArmVirtPkg/ArmVirtKvmTool.fdf
new file mode 100644
index 0000000000000000000000000000000000000000..ac014026c4fba0886c13c58c9ffeb23fea7c4606
--- /dev/null
+++ b/ArmVirtPkg/ArmVirtKvmTool.fdf
@@ -0,0 +1,297 @@
+#
+# Copyright (c) 2018, ARM Limited. All rights reserved.
+#
+# This program and the accompanying materials
+# are licensed and made available under the terms and conditions of the BSD License
+# which accompanies this distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+
+################################################################################
+#
+# FD Section
+# The [FD] Section is made up of the definition statements and a
+# description of what goes into the Flash Device Image. Each FD section
+# defines one flash "device" image. A flash device image may be one of
+# the following: Removable media bootable image (like a boot floppy
+# image,) an Option ROM image (that would be "flashed" into an add-in
+# card,) a System "Flash" image (that would be burned into a system's
+# flash) or an Update ("Capsule") image that will be used to update and
+# existing system flash.
+#
+################################################################################
+
+[FD.KVMTOOL_EFI]
+BaseAddress = 0x00000000|gArmTokenSpaceGuid.PcdFdBaseAddress # QEMU assigns 0 - 0x8000000 for a BootROM
+Size = 0x00200000|gArmTokenSpaceGuid.PcdFdSize # The size in bytes of the FLASH Device
+ErasePolarity = 1
+
+# This one is tricky, it must be: BlockSize * NumBlocks = Size
+BlockSize = 0x00001000
+NumBlocks = 0x200
+
+################################################################################
+#
+# Following are lists of FD Region layout which correspond to the locations of different
+# images within the flash device.
+#
+# Regions must be defined in ascending order and may not overlap.
+#
+# A Layout Region start with a eight digit hex offset (leading "0x" required) followed by
+# the pipe "|" character, followed by the size of the region, also in hex with the leading
+# "0x" characters. Like:
+# Offset|Size
+# PcdOffsetCName|PcdSizeCName
+# RegionType <FV, DATA, or FILE>
+#
+################################################################################
+
+#
+# Implement the Linux kernel header layout so that the loader will identify
+# it as something bootable, and execute it with a FDT pointer in x0 or r2.
+# This area will be reused to store a copy of the FDT so round it up to 32 KB.
+#
+0x00000000|0x00008000
+DATA = {
+!if $(ARCH) == AARCH64
+ 0x01, 0x00, 0x00, 0x10, # code0: adr x1, .
+ 0xff, 0x1f, 0x00, 0x14, # code1: b 0x8000
+ 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, # text_offset: 512 KB
+ 0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, # image_size: 2 MB
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # flags
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # res2
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # res3
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # res4
+ 0x41, 0x52, 0x4d, 0x64, # magic: "ARM\x64"
+ 0x00, 0x00, 0x00, 0x00 # res5
+!else
+ 0x08, 0x10, 0x4f, 0xe2, # adr r1, .
+ 0x02, 0x00, 0xa0, 0xe1, # mov r0, r2 (DTB)
+ 0x00, 0x00, 0xa0, 0xe1, # nop
+ 0x00, 0x00, 0xa0, 0xe1, # nop
+ 0x00, 0x00, 0xa0, 0xe1, # nop
+ 0x00, 0x00, 0xa0, 0xe1, # nop
+ 0x00, 0x00, 0xa0, 0xe1, # nop
+ 0x00, 0x00, 0xa0, 0xe1, # nop
+
+ 0xf6, 0x1f, 0x00, 0xea, # b 0x8000
+ 0x18, 0x28, 0x6f, 0x01, # magic
+ 0x00, 0x00, 0x00, 0x00, # start
+ 0x00, 0x00, 0x20, 0x00, # image size: 2 MB
+ 0x01, 0x02, 0x03, 0x04 # endiannness flag
+!endif
+}
+
+0x00008000|0x001f8000
+gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
+FV = FVMAIN_COMPACT
+
+!include VarStore.fdf.inc
+
+################################################################################
+#
+# FV Section
+#
+# [FV] section is used to define what components or modules are placed within a flash
+# device file. This section also defines order the components and modules are positioned
+# within the image. The [FV] section consists of define statements, set statements and
+# module statements.
+#
+################################################################################
+
+[FV.FvMain]
+FvNameGuid = 64074afe-340a-4be6-94ba-91b5b4d0f71e
+BlockSize = 0x40
+NumBlocks = 0 # This FV gets compressed so make it just big enough
+FvAlignment = 16 # FV alignment and FV attributes setting.
+ERASE_POLARITY = 1
+MEMORY_MAPPED = TRUE
+STICKY_WRITE = TRUE
+LOCK_CAP = TRUE
+LOCK_STATUS = TRUE
+WRITE_DISABLED_CAP = TRUE
+WRITE_ENABLED_CAP = TRUE
+WRITE_STATUS = TRUE
+WRITE_LOCK_CAP = TRUE
+WRITE_LOCK_STATUS = TRUE
+READ_DISABLED_CAP = TRUE
+READ_ENABLED_CAP = TRUE
+READ_STATUS = TRUE
+READ_LOCK_CAP = TRUE
+READ_LOCK_STATUS = TRUE
+
+ INF MdeModulePkg/Core/Dxe/DxeMain.inf
+ INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
+ INF ArmVirtPkg/VirtioFdtDxe/VirtioFdtDxe.inf
+ INF ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
+ INF ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
+ INF ArmVirtPkg/HighMemDxe/HighMemDxe.inf
+
+ #
+ # PI DXE Drivers producing Architectural Protocols (EFI Services)
+ #
+ INF ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+ INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
+ INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
+ INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
+ INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
+ INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
+
+!if $(SECURE_BOOT_ENABLE) == TRUE
+ INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+!endif
+ INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
+ INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
+
+ INF MdeModulePkg/Universal/Metronome/Metronome.inf
+ INF PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
+
+ INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
+
+ #
+ # Multiple Console IO support
+ #
+ INF MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
+ INF MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
+ INF MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
+ INF MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
+ INF MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
+
+ INF ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+ INF ArmPkg/Drivers/TimerDxe/TimerDxe.inf
+ INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
+
+ #
+ # FAT filesystem + GPT/MBR partitioning + UDF filesystem
+ #
+ INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
+ INF MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
+ INF FatPkg/EnhancedFatDxe/Fat.inf
+ INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
+ INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
+
+ #
+ # Platform Driver
+ #
+ INF OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
+ INF OvmfPkg/VirtioNetDxe/VirtioNet.inf
+ INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
+ INF OvmfPkg/VirtioRngDxe/VirtioRng.inf
+
+ #
+ # UEFI application (Shell Embedded Boot Loader)
+ #
+ INF ShellPkg/Application/Shell/Shell.inf
+ INF ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
+
+ #
+ # Bds
+ #
+ INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
+ INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
+ INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+ INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
+ INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+ INF MdeModulePkg/Application/UiApp/UiApp.inf
+
+!if $(ENABLE_NETWORK) == TRUE
+ #
+ # Networking stack
+ #
+ INF MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
+ INF MdeModulePkg/Universal/Network/DpcDxe/DpcDxe.inf
+ INF MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
+ INF MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
+ INF MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
+ INF MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
+ INF MdeModulePkg/Universal/Network/VlanConfigDxe/VlanConfigDxe.inf
+ INF MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Dxe.inf
+ INF MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
+ INF MdeModulePkg/Universal/Network/Udp4Dxe/Udp4Dxe.inf
+ INF MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
+ INF MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
+!if $(HTTP_BOOT_ENABLE) == TRUE
+ INF NetworkPkg/DnsDxe/DnsDxe.inf
+ INF NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf
+ INF NetworkPkg/HttpDxe/HttpDxe.inf
+ INF NetworkPkg/HttpBootDxe/HttpBootDxe.inf
+!endif
+!endif
+ #
+ # SCSI Bus and Disk Driver
+ #
+ INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
+ INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
+
+!if $(ARCH) == AARCH64
+ #
+ # EBC support
+ #
+ INF MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
+!endif
+
+ #
+ # PCI support
+ #
+ INF ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
+ INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+ INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+ INF OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
+ INF OvmfPkg/Virtio10Dxe/Virtio10.inf
+
+ #
+ # Video support
+ #
+ INF OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
+ INF OvmfPkg/PlatformDxe/Platform.inf
+
+ #
+ # USB Support
+ #
+ INF MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
+ INF MdeModulePkg/Bus/Pci/EhciDxe/EhciDxe.inf
+ INF MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
+ INF MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
+ INF MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
+ INF MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
+
+ #
+ # TianoCore logo (splash screen)
+ #
+ INF MdeModulePkg/Logo/LogoDxe.inf
+
+ #
+ # Ramdisk support
+ #
+ INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
+
+[FV.FVMAIN_COMPACT]
+FvAlignment = 16
+ERASE_POLARITY = 1
+MEMORY_MAPPED = TRUE
+STICKY_WRITE = TRUE
+LOCK_CAP = TRUE
+LOCK_STATUS = TRUE
+WRITE_DISABLED_CAP = TRUE
+WRITE_ENABLED_CAP = TRUE
+WRITE_STATUS = TRUE
+WRITE_LOCK_CAP = TRUE
+WRITE_LOCK_STATUS = TRUE
+READ_DISABLED_CAP = TRUE
+READ_ENABLED_CAP = TRUE
+READ_STATUS = TRUE
+READ_LOCK_CAP = TRUE
+READ_LOCK_STATUS = TRUE
+
+ INF ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
+
+ FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
+ SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
+ SECTION FV_IMAGE = FVMAIN
+ }
+ }
+
+!include ArmVirtRules.fdf.inc
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v1 6/6] ArmVirtPkg: Support for kvmtool emulated platform
2018-10-12 14:40 ` [PATCH v1 6/6] ArmVirtPkg: Support for kvmtool emulated platform Sami Mujawar
@ 2018-10-13 21:57 ` Laszlo Ersek
0 siblings, 0 replies; 21+ messages in thread
From: Laszlo Ersek @ 2018-10-13 21:57 UTC (permalink / raw)
To: Sami Mujawar, edk2-devel; +Cc: nd, Stephanie.Hughes-Fitt
On 10/12/18 16:40, Sami Mujawar wrote:
> Kvmtool is a virtual machine manager that enables hosting
> KVM guests. Kvmtool emulates certain devices like serial
> port, RTC, etc. essentially providing an emulated platform.
>
> This patch adds support for kvmtool emulated platform.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
> The changes can be seen at https://github.com/samimujawar/edk2/commit/f13ce7024f7216e4036459bab6ce4b1d700f6050
>
> Notes:
> v1:
> - Add support for Kvmtool emulated platform [SAMI]
>
> ArmVirtPkg/ArmVirtKvmTool.dsc | 390 ++++++++++++++++++++
> ArmVirtPkg/ArmVirtKvmTool.fdf | 297 +++++++++++++++
> 2 files changed, 687 insertions(+)
I'd like to see more justification for this platform (see my questions
under 0/6), and once those are provided, I would like the maintainership
of this new platform to be captured formally in Maintainers.txt.
(There is a community topic at
<https://github.com/tianocore/tianocore.github.io/wiki/Community-Virtual-Meetings>
about this in general, "Adding more detail to Maintainers.txt". The idea
is to adopt the pathname/pattern-based maintainer association from other
projects, like Linux and QEMU.)
Thanks
Laszlo
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/6] Add kvmtool emulated platform support for ARM
2018-10-12 14:40 [PATCH 0/6] Add kvmtool emulated platform support for ARM Sami Mujawar
` (5 preceding siblings ...)
2018-10-12 14:40 ` [PATCH v1 6/6] ArmVirtPkg: Support for kvmtool emulated platform Sami Mujawar
@ 2018-10-13 21:42 ` Laszlo Ersek
2018-10-16 3:00 ` Leif Lindholm
6 siblings, 1 reply; 21+ messages in thread
From: Laszlo Ersek @ 2018-10-13 21:42 UTC (permalink / raw)
To: Sami Mujawar, edk2-devel
Cc: ruiyu.ni, nd, Stephanie.Hughes-Fitt, star.zeng, Drew Jones,
Ard Biesheuvel, Eric Auger
On 10/12/18 16:40, Sami Mujawar wrote:
> Kvmtool is a virtual machine manager that enables hosting KVM
> guests. ARM is working to enhance kvmtool support to enable
> launching of KVM guest with UEFI support.
Why is QEMU not good enough? (With or without KVM.)
Another platform I've recently learned about (for QEMU) is the SBSA
reference machine type. I'm concerned that this kind of divergence will
be hard to maintain in a common firmware package. Here's my understanding:
- ArmVirtQemu: supposed to run data center / cloud workloads
- SBSA reference machine: supposed to emulate physical machines as
closely as possible; primarily intended as a development environment
for physical machines
- firmware for the kvmtool platform: ?
> This patch series enables UEFI support for kvmtool's emulated ARM
> platform and is required to allow testing of kvmtool enhancements.
>
> Note:
> 1. These kvmtool platform support patches currently expose
> the kvmtool provided DT to the OS. Support for ACPI is
> planned, using Dynamic Tables Framework and should be
> available in the near future.
The Dynamic ACPI Tables Framework is not useful under QEMU, because QEMU
generates its own set of tables.
The framework would be very useful (I think) on the SBSA reference
machine, given that the framework would be shipped (presumably) on
physical machines as well. Why is it useful for kvmtool?
> 2. This new platform port can only be used with the updated
> kvmtool that is currently under development review.
>
> The changes can be seen at https://github.com/samimujawar/edk2/tree/299_kvmtool_plat_support_v1
I think it's valid to develop and review both streams of patches in
parallel. Usually firmware maintainers ask for the hardware emulation
bits to be pushed first.
Thanks
Laszlo
>
> Sami Mujawar (6):
> PcAtChipsetPkg: Add MMIO Support to SerialIo Lib
> PcAtChipsetPkg: Add MMIO Support to RTC driver
> MdeModulePkg: Map persistent (NV) memory
> ArmVirtPkg: Save DT base address from X0 in PCD
> ArmVirtPkg: Add kvmtool platform driver
> ArmVirtPkg: Support for kvmtool emulated platform
>
> ArmVirtPkg/ArmVirtKvmTool.dsc | 390 ++++++++++++++++++++
> ArmVirtPkg/ArmVirtKvmTool.fdf | 297 +++++++++++++++
> ArmVirtPkg/ArmVirtPkg.dec | 12 +-
> ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c | 211 +++++++++++
> ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf | 60 +++
> ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S | 9 +-
> MdeModulePkg/MdeModulePkg.dec | 9 +
> MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c | 77 +++-
> MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf | 6 +
> PcAtChipsetPkg/Library/SerialIoLib/SerialIoLib.inf | 4 +
> PcAtChipsetPkg/Library/SerialIoLib/SerialPortLib.c | 98 ++++-
> PcAtChipsetPkg/PcAtChipsetPkg.dec | 8 +
> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 38 +-
> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtcEntry.c | 112 +++++-
> PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf | 8 +
> 15 files changed, 1309 insertions(+), 30 deletions(-)
> create mode 100644 ArmVirtPkg/ArmVirtKvmTool.dsc
> create mode 100644 ArmVirtPkg/ArmVirtKvmTool.fdf
> create mode 100644 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.c
> create mode 100644 ArmVirtPkg/KvmtoolPlatformDxe/KvmtoolPlatformDxe.inf
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 0/6] Add kvmtool emulated platform support for ARM
2018-10-13 21:42 ` [PATCH 0/6] Add kvmtool emulated platform support for ARM Laszlo Ersek
@ 2018-10-16 3:00 ` Leif Lindholm
0 siblings, 0 replies; 21+ messages in thread
From: Leif Lindholm @ 2018-10-16 3:00 UTC (permalink / raw)
To: Laszlo Ersek
Cc: Sami Mujawar, edk2-devel, ruiyu.ni, Drew Jones,
Stephanie.Hughes-Fitt, Eric Auger, nd, star.zeng
On Sat, Oct 13, 2018 at 11:42:00PM +0200, Laszlo Ersek wrote:
> On 10/12/18 16:40, Sami Mujawar wrote:
> > Kvmtool is a virtual machine manager that enables hosting KVM
> > guests. ARM is working to enhance kvmtool support to enable
> > launching of KVM guest with UEFI support.
>
> Why is QEMU not good enough? (With or without KVM.)
>
> Another platform I've recently learned about (for QEMU) is the SBSA
> reference machine type. I'm concerned that this kind of divergence will
> be hard to maintain in a common firmware package. Here's my understanding:
>
> - ArmVirtQemu: supposed to run data center / cloud workloads
> - SBSA reference machine: supposed to emulate physical machines as
> closely as possible; primarily intended as a development environment
> for physical machines
If it helps - try to not think of SBSA QEMU as a QEMU target.
It's pretending to be a hardware platform and should be treated as
such. While we may have started the firmware port from ArmVirtPkg (as
the qemu machine was being developed), I don't expect it to ultimately
end up there when it goes upstream.
Regards,
Leif
^ permalink raw reply [flat|nested] 21+ messages in thread