|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 21/29] Ovmf/Xen: move XenBusDxe to abstract XENIO_PROTOCOL
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 <ard.biesheuvel@xxxxxxxxxx>
> ---
> 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 <lersek@xxxxxxxxxx>
Thanks
Laszlo
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |