The patch extends VIRTIO_DEVICE_PROTOCOL to provide the following new
member functions:
- AllocateSharedPages : allocate a memory region suitable for sharing
between guest and hypervisor (e.g ring buffer).
- FreeSharedPages: free the memory allocated using AllocateSharedPages ().
- MapSharedBuffer: map a host address to device address suitable to share
with device for bus master operations.
- UnmapSharedBuffer: unmap the device address obtained through the
MapSharedBuffer().
Suggested-by: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
OvmfPkg/Include/Protocol/VirtioDevice.h | 143 ++++++++++++++++++++
1 file changed, 143 insertions(+)
diff --git a/OvmfPkg/Include/Protocol/VirtioDevice.h b/OvmfPkg/Include/Protocol/VirtioDevice.h
index edb20c18822c..14f980d7bf0a 100644
--- a/OvmfPkg/Include/Protocol/VirtioDevice.h
+++ b/OvmfPkg/Include/Protocol/VirtioDevice.h
@@ -5,6 +5,7 @@
and should not be used outside of the EDK II tree.
Copyright (c) 2013, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2017, AMD Inc, 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
@@ -31,6 +32,25 @@
typedef struct _VIRTIO_DEVICE_PROTOCOL VIRTIO_DEVICE_PROTOCOL;
+//
+// VIRTIO Operation for VIRTIO_MAP_SHARED
+//
+typedef enum {
+ //
+ // A read operation from system memory by a bus master
+ //
+ VirtioOperationBusMasterRead,
+ //
+ // A write operation to system memory by a bus master
+ //
+ VirtioOperationBusMasterWrite,
+ //
+ // Provides both read and write access to system memory by both the
+ // processor and a bus master
+ //
+ VirtioOperationBusMasterCommonBuffer,
+} VIRTIO_MAP_OPERATION;
+
/**
Read a word from the device-specific I/O region of the Virtio Header.
@@ -319,6 +339,121 @@ EFI_STATUS
IN UINT8 DeviceStatus
);
+/**
+
+ Allocates pages that are suitable for an VirtioOperationBusMasterCommonBuffer
+ mapping. This means that the buffer allocated by this function supports
+ simultaneous access by both the processor and the bus master. The device
+ address that the bus master uses to access the buffer must be retrieved with
+ a call to VIRTIO_MAP_SHARED.
+
+ @param[in] This The protocol instance pointer.
+
+ @param[in] Pages The number of pages to allocate.
+
+ @param[in,out] HostAddress A pointer to store the system memory base
+ address of the allocated range.
+
+ @retval EFI_SUCCESS The requested memory pages were allocated.
+ @retval EFI_OUT_OF_RESOURCES The memory pages could not be allocated.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *VIRTIO_ALLOCATE_SHARED)(
+ IN VIRTIO_DEVICE_PROTOCOL *This,
+ IN UINTN Pages,
+ IN OUT VOID **HostAddress
+ );
+
+/**
+ Frees memory that was allocated with VIRTIO_ALLOCATE_SHARED.
+
+ @param[in] This The protocol instance pointer.
+
+ @param[in] Pages The number of pages to free.
+
+ @param[in] HostAddress The system memory base address of the allocated
+ range.
+
+**/
+typedef
+VOID
+(EFIAPI *VIRTIO_FREE_SHARED)(
+ IN VIRTIO_DEVICE_PROTOCOL *This,
+ IN UINTN Pages,
+ IN VOID *HostAddress
+ );
+
+/**
+ Provides the virtio device address required to access system memory from a
+ DMA bus master.
+
+ The interface follows the same usage pattern as defined in UEFI spec 2.6
+ (Section 13.2 PCI Root Bridge I/O Protocol)
+
+ @param[in] This The protocol instance pointer.
+
+ @param[in] Operation Indicates if the bus master is going to
+ read or write to system memory.
+
+ @param[in] HostAddress The system memory address to map to shared
+ buffer address.
+
+ @param[in,out] NumberOfBytes On input the number of bytes to map.
+ On output the number of bytes that were
+ mapped.
+
+ @param[out] DeviceAddress The resulting shared map address for the
+ bus master to access the hosts HostAddress.
+
+ @param[out] Mapping A resulting taken to pass to
+ VIRTIO_UNMAP_SHARED.
+
+ @retval EFI_SUCCESS The range was mapped for the returned
+ NumberOfBytes.
+ @retval EFI_UNSUPPORTED The HostAddress cannot be mapped as a
+ common buffer.
+ @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+ @retval EFI_OUT_OF_RESOURCES The request could not be completed due to
+ a lack of resources.
+ @retval EFI_DEVICE_ERROR The system hardware could not map the
+ requested address.
+**/
+
+typedef
+EFI_STATUS
+(EFIAPI *VIRTIO_MAP_SHARED) (
+ IN VIRTIO_DEVICE_PROTOCOL *This,
+ IN VIRTIO_MAP_OPERATION Operation,
+ IN VOID *HostAddress,
+ IN OUT UINTN *NumberOfBytes,
+ OUT EFI_PHYSICAL_ADDRESS *DeviceAddress,
+ OUT VOID **Mapping
+ );
+
+/**
+ Completes the VIRTIO_MAP_SHARED operation and releases any corresponding
+ resources.
+
+ @param[in] This The protocol instance pointer.
+
+ @param[in] Mapping The mapping token returned from
+ VIRTIO_MAP_SHARED.
+
+ @retval EFI_SUCCESS The range was unmapped.
+ @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by
+ VIRTIO_MAP_SHARED. Passing an invalid Mapping
+ token can cause undefined behavior.
+ @retval EFI_DEVICE_ERROR The data was not committed to the target
+ system memory.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *VIRTIO_UNMAP_SHARED)(
+ IN VIRTIO_DEVICE_PROTOCOL *This,
+ IN VOID *Mapping
+ );
/**
@@ -359,6 +494,14 @@ struct _VIRTIO_DEVICE_PROTOCOL {
// Functions to read/write Device Specific headers
VIRTIO_DEVICE_WRITE WriteDevice;
VIRTIO_DEVICE_READ ReadDevice;
+
+ //
+ // Functions to allocate, free, map and unmap shared buffer
+ //
+ VIRTIO_ALLOCATE_SHARED AllocateSharedPages;
+ VIRTIO_FREE_SHARED FreeSharedPages;
+ VIRTIO_MAP_SHARED MapSharedBuffer;
+ VIRTIO_UNMAP_SHARED UnmapSharedBuffer;
};
extern EFI_GUID gVirtioDeviceProtocolGuid;
--
2.7.4
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
(1) In my previous review (msgid
<f953ec03-b160-726b-b9ed-5e3122642815@redhat.com>) point (1), I
suggested the following subject line:
"OvmfPkg: introduce IOMMU-like member functions to VIRTIO_DEVICE_PROTOCOL"
This subject was 72 characters long (within the 74 chars limit).
Your current subject is:
"OvmfPkg/Virtio: introduce IOMMU-like member functions to
VIRTIO_DEVICE_PROTOCOL"
It is too long (79 chars).
So please drop the "/Virtio" string, as I requested.
On 08/14/17 13:36, Brijesh Singh wrote:
> The patch extends VIRTIO_DEVICE_PROTOCOL to provide the following new
> member functions:
>
> - AllocateSharedPages : allocate a memory region suitable for sharing
> between guest and hypervisor (e.g ring buffer).
>
> - FreeSharedPages: free the memory allocated using AllocateSharedPages ().
>
> - MapSharedBuffer: map a host address to device address suitable to share
> with device for bus master operations.
>
> - UnmapSharedBuffer: unmap the device address obtained through the
> MapSharedBuffer().
(2) You missed point (20) of my above-referenced v1 review. Again,
please append the following to the commit message:
---------
We're free to extend the protocol structure without changing the
protocol GUID, or bumping any protocol version fields (of which we
currently have none), because VIRTIO_DEVICE_PROTOCOL is internal to edk2
by design -- see the disclaimers in "VirtioDevice.h".
---------
>
> Suggested-by: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> ---
> OvmfPkg/Include/Protocol/VirtioDevice.h | 143 ++++++++++++++++++++
> 1 file changed, 143 insertions(+)
>
> diff --git a/OvmfPkg/Include/Protocol/VirtioDevice.h b/OvmfPkg/Include/Protocol/VirtioDevice.h
> index edb20c18822c..14f980d7bf0a 100644
> --- a/OvmfPkg/Include/Protocol/VirtioDevice.h
> +++ b/OvmfPkg/Include/Protocol/VirtioDevice.h
> @@ -5,6 +5,7 @@
> and should not be used outside of the EDK II tree.
>
> Copyright (c) 2013, ARM Ltd. All rights reserved.<BR>
> + Copyright (c) 2017, AMD Inc, 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
> @@ -31,6 +32,25 @@
>
> typedef struct _VIRTIO_DEVICE_PROTOCOL VIRTIO_DEVICE_PROTOCOL;
>
> +//
> +// VIRTIO Operation for VIRTIO_MAP_SHARED
> +//
> +typedef enum {
> + //
> + // A read operation from system memory by a bus master
> + //
> + VirtioOperationBusMasterRead,
> + //
> + // A write operation to system memory by a bus master
> + //
> + VirtioOperationBusMasterWrite,
> + //
> + // Provides both read and write access to system memory by both the
> + // processor and a bus master
> + //
> + VirtioOperationBusMasterCommonBuffer,
> +} VIRTIO_MAP_OPERATION;
> +
> /**
>
> Read a word from the device-specific I/O region of the Virtio Header.
> @@ -319,6 +339,121 @@ EFI_STATUS
> IN UINT8 DeviceStatus
> );
>
> +/**
> +
> + Allocates pages that are suitable for an VirtioOperationBusMasterCommonBuffer
> + mapping. This means that the buffer allocated by this function supports
> + simultaneous access by both the processor and the bus master. The device
> + address that the bus master uses to access the buffer must be retrieved with
> + a call to VIRTIO_MAP_SHARED.
> +
> + @param[in] This The protocol instance pointer.
> +
> + @param[in] Pages The number of pages to allocate.
> +
> + @param[in,out] HostAddress A pointer to store the system memory base
> + address of the allocated range.
> +
> + @retval EFI_SUCCESS The requested memory pages were allocated.
> + @retval EFI_OUT_OF_RESOURCES The memory pages could not be allocated.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *VIRTIO_ALLOCATE_SHARED)(
> + IN VIRTIO_DEVICE_PROTOCOL *This,
> + IN UINTN Pages,
> + IN OUT VOID **HostAddress
> + );
> +
> +/**
> + Frees memory that was allocated with VIRTIO_ALLOCATE_SHARED.
> +
> + @param[in] This The protocol instance pointer.
> +
> + @param[in] Pages The number of pages to free.
> +
> + @param[in] HostAddress The system memory base address of the allocated
> + range.
> +
> +**/
> +typedef
> +VOID
> +(EFIAPI *VIRTIO_FREE_SHARED)(
> + IN VIRTIO_DEVICE_PROTOCOL *This,
> + IN UINTN Pages,
> + IN VOID *HostAddress
> + );
> +
> +/**
> + Provides the virtio device address required to access system memory from a
> + DMA bus master.
> +
> + The interface follows the same usage pattern as defined in UEFI spec 2.6
> + (Section 13.2 PCI Root Bridge I/O Protocol)
> +
> + @param[in] This The protocol instance pointer.
> +
> + @param[in] Operation Indicates if the bus master is going to
> + read or write to system memory.
> +
> + @param[in] HostAddress The system memory address to map to shared
> + buffer address.
> +
> + @param[in,out] NumberOfBytes On input the number of bytes to map.
> + On output the number of bytes that were
> + mapped.
> +
> + @param[out] DeviceAddress The resulting shared map address for the
> + bus master to access the hosts HostAddress.
> +
> + @param[out] Mapping A resulting taken to pass to
(3) s/taken/token/
The rest looks fine!
Thanks,
Laszlo
> + VIRTIO_UNMAP_SHARED.
> +
> + @retval EFI_SUCCESS The range was mapped for the returned
> + NumberOfBytes.
> + @retval EFI_UNSUPPORTED The HostAddress cannot be mapped as a
> + common buffer.
> + @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
> + @retval EFI_OUT_OF_RESOURCES The request could not be completed due to
> + a lack of resources.
> + @retval EFI_DEVICE_ERROR The system hardware could not map the
> + requested address.
> +**/
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *VIRTIO_MAP_SHARED) (
> + IN VIRTIO_DEVICE_PROTOCOL *This,
> + IN VIRTIO_MAP_OPERATION Operation,
> + IN VOID *HostAddress,
> + IN OUT UINTN *NumberOfBytes,
> + OUT EFI_PHYSICAL_ADDRESS *DeviceAddress,
> + OUT VOID **Mapping
> + );
> +
> +/**
> + Completes the VIRTIO_MAP_SHARED operation and releases any corresponding
> + resources.
> +
> + @param[in] This The protocol instance pointer.
> +
> + @param[in] Mapping The mapping token returned from
> + VIRTIO_MAP_SHARED.
> +
> + @retval EFI_SUCCESS The range was unmapped.
> + @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by
> + VIRTIO_MAP_SHARED. Passing an invalid Mapping
> + token can cause undefined behavior.
> + @retval EFI_DEVICE_ERROR The data was not committed to the target
> + system memory.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *VIRTIO_UNMAP_SHARED)(
> + IN VIRTIO_DEVICE_PROTOCOL *This,
> + IN VOID *Mapping
> + );
>
> /**
>
> @@ -359,6 +494,14 @@ struct _VIRTIO_DEVICE_PROTOCOL {
> // Functions to read/write Device Specific headers
> VIRTIO_DEVICE_WRITE WriteDevice;
> VIRTIO_DEVICE_READ ReadDevice;
> +
> + //
> + // Functions to allocate, free, map and unmap shared buffer
> + //
> + VIRTIO_ALLOCATE_SHARED AllocateSharedPages;
> + VIRTIO_FREE_SHARED FreeSharedPages;
> + VIRTIO_MAP_SHARED MapSharedBuffer;
> + VIRTIO_UNMAP_SHARED UnmapSharedBuffer;
> };
>
> extern EFI_GUID gVirtioDeviceProtocolGuid;
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
On 08/16/2017 09:37 AM, Laszlo Ersek wrote: > (1) In my previous review (msgid > <f953ec03-b160-726b-b9ed-5e3122642815@redhat.com>) point (1), I > suggested the following subject line: > > "OvmfPkg: introduce IOMMU-like member functions to VIRTIO_DEVICE_PROTOCOL" > > This subject was 72 characters long (within the 74 chars limit). > > Your current subject is: > > "OvmfPkg/Virtio: introduce IOMMU-like member functions to > VIRTIO_DEVICE_PROTOCOL" > > It is too long (79 chars). > > So please drop the "/Virtio" string, as I requested. > Will do > On 08/14/17 13:36, Brijesh Singh wrote: >> The patch extends VIRTIO_DEVICE_PROTOCOL to provide the following new >> member functions: >> >> - AllocateSharedPages : allocate a memory region suitable for sharing >> between guest and hypervisor (e.g ring buffer). >> >> - FreeSharedPages: free the memory allocated using AllocateSharedPages (). >> >> - MapSharedBuffer: map a host address to device address suitable to share >> with device for bus master operations. >> >> - UnmapSharedBuffer: unmap the device address obtained through the >> MapSharedBuffer(). > > (2) You missed point (20) of my above-referenced v1 review. Again, > please append the following to the commit message: > > --------- > We're free to extend the protocol structure without changing the > protocol GUID, or bumping any protocol version fields (of which we > currently have none), because VIRTIO_DEVICE_PROTOCOL is internal to edk2 > by design -- see the disclaimers in "VirtioDevice.h". > --------- > I will add this text in commit message. [snip] > > (3) s/taken/token/ I think I will have his error in all the place (due to copy/paste). I will go through each patches and fix them all. -Brijesh _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
© 2016 - 2024 Red Hat, Inc.