comments below
On 01/26/15 20:03, Ard Biesheuvel wrote:
> While Xen on Intel uses a virtual PCI device to communicate the
> base address of the grant table, the ARM implementation uses a DT
> node, which is fundamentally incompatible with the way XenBusDxe is
> implemented, i.e., as a UEFI Driver Model implementation for a PCI
> device.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32.fdf | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.fdf | 1 +
> OvmfPkg/XenBusDxe/ComponentName.c | 2 +-
> OvmfPkg/XenBusDxe/GrantTable.c | 5 ++---
> OvmfPkg/XenBusDxe/GrantTable.h | 3 +--
> OvmfPkg/XenBusDxe/XenBus.c | 6 +++---
> OvmfPkg/XenBusDxe/XenBusDxe.c | 57
> ++++++++++++++-------------------------------------------
> OvmfPkg/XenBusDxe/XenBusDxe.h | 8 ++------
> OvmfPkg/XenBusDxe/XenBusDxe.inf | 2 +-
> 13 files changed, 30 insertions(+), 59 deletions(-)
>
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 90540272745c..8c880613851d 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -444,6 +444,7 @@
> OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
> OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
> OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
> + OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf
> OvmfPkg/XenBusDxe/XenBusDxe.inf
> OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf {
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index f47e7ddc7834..ecef963d1e85 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -227,6 +227,7 @@ INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
> INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
> INF OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
> INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +INF OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf
> INF OvmfPkg/XenBusDxe/XenBusDxe.inf
> INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
>
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 0a331eda8be0..ff32ecefd07b 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -451,6 +451,7 @@
> OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
> OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
> OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
> + OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf
> OvmfPkg/XenBusDxe/XenBusDxe.inf
> OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf {
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index 4c034460d5d2..29414ff04082 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -227,6 +227,7 @@ INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
> INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
> INF OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
> INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +INF OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf
> INF OvmfPkg/XenBusDxe/XenBusDxe.inf
> INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
>
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index e2b37c271681..8bac6dc313f0 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -449,6 +449,7 @@
> OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
> OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
> OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
> + OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf
> OvmfPkg/XenBusDxe/XenBusDxe.inf
> OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf {
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 2323eb2edf33..f1feb698ba66 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -227,6 +227,7 @@ INF OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
> INF OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
> INF OvmfPkg/EmuVariableFvbRuntimeDxe/Fvb.inf
> INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +INF OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf
> INF OvmfPkg/XenBusDxe/XenBusDxe.inf
> INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
>
> diff --git a/OvmfPkg/XenBusDxe/ComponentName.c
> b/OvmfPkg/XenBusDxe/ComponentName.c
> index 4530509e65dc..3f2dd406c77d 100644
> --- a/OvmfPkg/XenBusDxe/ComponentName.c
> +++ b/OvmfPkg/XenBusDxe/ComponentName.c
> @@ -155,7 +155,7 @@ XenBusDxeComponentNameGetControllerName (
> Status = EfiTestManagedDevice (
> ControllerHandle,
> gXenBusDxeDriverBinding.DriverBindingHandle,
> - &gEfiPciIoProtocolGuid
> + &gXenIoProtocolGuid
> );
> if (EFI_ERROR (Status)) {
> return Status;
> diff --git a/OvmfPkg/XenBusDxe/GrantTable.c b/OvmfPkg/XenBusDxe/GrantTable.c
> index a80d5eff39cd..19117fbe0373 100644
> --- a/OvmfPkg/XenBusDxe/GrantTable.c
> +++ b/OvmfPkg/XenBusDxe/GrantTable.c
> @@ -139,8 +139,7 @@ XenGrantTableEndAccess (
>
> VOID
> XenGrantTableInit (
> - IN XENBUS_DEVICE *Dev,
> - IN UINT64 MmioAddr
> + IN XENBUS_DEVICE *Dev
> )
> {
> xen_add_to_physmap_t Parameters;
> @@ -155,7 +154,7 @@ XenGrantTableInit (
> XenGrantTablePutFreeEntry ((grant_ref_t)Index);
> }
>
> - GrantTable = (VOID*)(UINTN) MmioAddr;
> + GrantTable = (VOID*)(UINTN) Dev->XenIo->GrantTableAddress;
> for (Index = 0; Index < NR_GRANT_FRAMES; Index++) {
> Parameters.domid = DOMID_SELF;
> Parameters.idx = Index;
> diff --git a/OvmfPkg/XenBusDxe/GrantTable.h b/OvmfPkg/XenBusDxe/GrantTable.h
> index 5772c56662df..194275ba7ed5 100644
> --- a/OvmfPkg/XenBusDxe/GrantTable.h
> +++ b/OvmfPkg/XenBusDxe/GrantTable.h
> @@ -29,8 +29,7 @@
> **/
> VOID
> XenGrantTableInit (
> - IN XENBUS_DEVICE *Dev,
> - IN UINT64 MmioAddr
> + IN XENBUS_DEVICE *Dev
> );
>
> /**
> diff --git a/OvmfPkg/XenBusDxe/XenBus.c b/OvmfPkg/XenBusDxe/XenBus.c
> index f69c27dd184a..ee9526c33252 100644
> --- a/OvmfPkg/XenBusDxe/XenBus.c
> +++ b/OvmfPkg/XenBusDxe/XenBus.c
> @@ -138,7 +138,7 @@ XenBusAddDevice (
> XENBUS_PRIVATE_DATA *Private;
> EFI_STATUS Status;
> XENBUS_DEVICE_PATH *TempXenBusPath;
> - VOID *ChildPciIo;
> + VOID *ChildXenIo;
>
> AsciiSPrint (DevicePath, sizeof (DevicePath),
> "device/%a/%a", Type, Id);
> @@ -208,8 +208,8 @@ XenBusAddDevice (
> }
>
> Status = gBS->OpenProtocol (Dev->ControllerHandle,
> - &gEfiPciIoProtocolGuid,
> - &ChildPciIo, Dev->This->DriverBindingHandle,
> + &gXenIoProtocolGuid,
> + &ChildXenIo, Dev->This->DriverBindingHandle,
> Private->Handle,
> EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER);
> if (EFI_ERROR (Status)) {
> diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.c b/OvmfPkg/XenBusDxe/XenBusDxe.c
> index cc334c086c1f..6474428b79e5 100644
> --- a/OvmfPkg/XenBusDxe/XenBusDxe.c
> +++ b/OvmfPkg/XenBusDxe/XenBusDxe.c
> @@ -23,8 +23,6 @@
>
> **/
>
> -#include <IndustryStandard/Pci.h>
> -#include <IndustryStandard/Acpi.h>
> #include <Library/DebugLib.h>
> #include <Library/XenHypercallLib.h>
>
> @@ -233,35 +231,23 @@ XenBusDxeDriverBindingSupported (
> )
> {
> EFI_STATUS Status;
> - EFI_PCI_IO_PROTOCOL *PciIo;
> - PCI_TYPE00 Pci;
> + XENIO_PROTOCOL *XenIo;
>
> Status = gBS->OpenProtocol (
> ControllerHandle,
> - &gEfiPciIoProtocolGuid,
> - (VOID **)&PciIo,
> + &gXenIoProtocolGuid,
> + (VOID **)&XenIo,
> This->DriverBindingHandle,
> ControllerHandle,
> EFI_OPEN_PROTOCOL_BY_DRIVER
> );
> +
> if (EFI_ERROR (Status)) {
> return Status;
> }
>
> - Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint32, 0,
> - sizeof Pci / sizeof (UINT32), &Pci);
> -
> - if (Status == EFI_SUCCESS) {
> - if (Pci.Hdr.VendorId == PCI_VENDOR_ID_XEN &&
> - Pci.Hdr.DeviceId == PCI_DEVICE_ID_XEN_PLATFORM) {
> - Status = EFI_SUCCESS;
> - } else {
> - Status = EFI_UNSUPPORTED;
> - }
> - }
> -
> - gBS->CloseProtocol (ControllerHandle, &gEfiPciIoProtocolGuid,
> - This->DriverBindingHandle, ControllerHandle);
> + gBS->CloseProtocol (ControllerHandle, &gXenIoProtocolGuid,
> + This->DriverBindingHandle, ControllerHandle);
Whitespace damage. The parameter should be indented one or two spaces
relative to the called member.
>
> return Status;
> }
> @@ -326,19 +312,18 @@ XenBusDxeDriverBindingStart (
> {
> EFI_STATUS Status;
> XENBUS_DEVICE *Dev;
> - EFI_PCI_IO_PROTOCOL *PciIo;
> - EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR *BarDesc;
> - UINT64 MmioAddr;
> + XENIO_PROTOCOL *XenIo;
> EFI_DEVICE_PATH_PROTOCOL *DevicePath;
>
> Status = gBS->OpenProtocol (
> ControllerHandle,
> - &gEfiPciIoProtocolGuid,
> - (VOID **) &PciIo,
> + &gXenIoProtocolGuid,
> + (VOID**)&XenIo,
> This->DriverBindingHandle,
> ControllerHandle,
> EFI_OPEN_PROTOCOL_BY_DRIVER
> );
> +
> if (EFI_ERROR (Status)) {
> return Status;
> }
> @@ -360,7 +345,7 @@ XenBusDxeDriverBindingStart (
> Dev->Signature = XENBUS_DEVICE_SIGNATURE;
> Dev->This = This;
> Dev->ControllerHandle = ControllerHandle;
> - Dev->PciIo = PciIo;
> + Dev->XenIo = XenIo;
> Dev->DevicePath = DevicePath;
> InitializeListHead (&Dev->ChildList);
>
> @@ -376,20 +361,6 @@ XenBusDxeDriverBindingStart (
> mMyDevice = Dev;
> EfiReleaseLock (&mMyDeviceLock);
>
> - //
> - // The BAR1 of this PCI device is used for shared memory and is supposed to
> - // look like MMIO. The address space of the BAR1 will be used to map the
> - // Grant Table.
> - //
> - Status = PciIo->GetBarAttributes (PciIo, PCI_BAR_IDX1, NULL, (VOID**)
> &BarDesc);
> - ASSERT_EFI_ERROR (Status);
> - ASSERT (BarDesc->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM);
> -
> - /* Get a Memory address for mapping the Grant Table. */
> - DEBUG ((EFI_D_INFO, "XenBus: BAR at %LX\n", BarDesc->AddrRangeMin));
> - MmioAddr = BarDesc->AddrRangeMin;
> - FreePool (BarDesc);
> -
> Status = XenGetSharedInfoPage (Dev);
> if (EFI_ERROR (Status)) {
> DEBUG ((EFI_D_ERROR, "XenBus: Unable to get the shared info page.\n"));
> @@ -397,7 +368,7 @@ XenBusDxeDriverBindingStart (
> goto ErrorAllocated;
> }
>
> - XenGrantTableInit (Dev, MmioAddr);
> + XenGrantTableInit (Dev);
>
> Status = XenStoreInit (Dev);
> ASSERT_EFI_ERROR (Status);
> @@ -417,7 +388,7 @@ ErrorAllocated:
> gBS->CloseProtocol (ControllerHandle, &gEfiDevicePathProtocolGuid,
> This->DriverBindingHandle, ControllerHandle);
> ErrorOpenningProtocol:
> - gBS->CloseProtocol (ControllerHandle, &gEfiPciIoProtocolGuid,
> + gBS->CloseProtocol (ControllerHandle, &gXenIoProtocolGuid,
> This->DriverBindingHandle, ControllerHandle);
> return Status;
> }
> @@ -507,7 +478,7 @@ XenBusDxeDriverBindingStop (
>
> gBS->CloseProtocol (ControllerHandle, &gEfiDevicePathProtocolGuid,
> This->DriverBindingHandle, ControllerHandle);
> - gBS->CloseProtocol (ControllerHandle, &gEfiPciIoProtocolGuid,
> + gBS->CloseProtocol (ControllerHandle, &gXenIoProtocolGuid,
> This->DriverBindingHandle, ControllerHandle);
>
> mMyDevice = NULL;
> diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.h b/OvmfPkg/XenBusDxe/XenBusDxe.h
> index 9b7219906a69..6c306e017b07 100644
> --- a/OvmfPkg/XenBusDxe/XenBusDxe.h
> +++ b/OvmfPkg/XenBusDxe/XenBusDxe.h
> @@ -39,7 +39,7 @@
> //
> // Consumed Protocols
> //
> -#include <Protocol/PciIo.h>
> +#include <Protocol/XenIo.h>
>
>
> //
> @@ -73,10 +73,6 @@ extern EFI_COMPONENT_NAME_PROTOCOL
> gXenBusDxeComponentName;
> //
> #include <IndustryStandard/Xen/xen.h>
>
> -#define PCI_VENDOR_ID_XEN 0x5853
> -#define PCI_DEVICE_ID_XEN_PLATFORM 0x0001
> -
> -
> typedef struct _XENBUS_DEVICE_PATH XENBUS_DEVICE_PATH;
> typedef struct _XENBUS_DEVICE XENBUS_DEVICE;
>
> @@ -86,7 +82,7 @@ struct _XENBUS_DEVICE {
> UINT32 Signature;
> EFI_DRIVER_BINDING_PROTOCOL *This;
> EFI_HANDLE ControllerHandle;
> - EFI_PCI_IO_PROTOCOL *PciIo;
> + XENIO_PROTOCOL *XenIo;
> EFI_EVENT ExitBootEvent;
> EFI_DEVICE_PATH_PROTOCOL *DevicePath;
> LIST_ENTRY ChildList;
> diff --git a/OvmfPkg/XenBusDxe/XenBusDxe.inf b/OvmfPkg/XenBusDxe/XenBusDxe.inf
> index 714607dbd6f8..31553ac5a64a 100644
> --- a/OvmfPkg/XenBusDxe/XenBusDxe.inf
> +++ b/OvmfPkg/XenBusDxe/XenBusDxe.inf
> @@ -67,8 +67,8 @@
>
> [Protocols]
> gEfiDriverBindingProtocolGuid
> - gEfiPciIoProtocolGuid
> gEfiComponentName2ProtocolGuid
> gEfiComponentNameProtocolGuid
> gXenBusProtocolGuid
> + gXenIoProtocolGuid
>
>
I won't "audit" XenBusDxe, obviously, but if you covered all PciIo
occurrences with the above, then it should not regress anything.
Please restore the indentation.
Acked-by: Laszlo Ersek <[email protected]>
Thanks
Laszlo
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
