On 08/14/17 13:36, Brijesh Singh wrote: > The patch implements the newly added IOMMU-like member functions by > respectively delegating the job to: > > - VIRTIO_DEVICE_PROTOCOL.AllocateSharedPages() -> > EFI_PCI_IO_PROTOCOL.AllocateBuffer() > > - VIRTIO_DEVICE_PROTOCOL.FreeSharedPages() -> > EFI_PCI_IO_PROTOCOL.FreeBuffer() > > - VIRTIO_DEVICE_PROTOCOL.MapSharedBuffer() -> > EFI_PCI_IO_PROTOCOL.Map() > > - VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() -> > EFI_PCI_IO_PROTOCOL.Unmap() > > Suggested-by: Laszlo Ersek <[email protected]> > Cc: Ard Biesheuvel <[email protected]> > Cc: Jordan Justen <[email protected]> > Cc: Tom Lendacky <[email protected]> > Cc: Laszlo Ersek <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Brijesh Singh <[email protected]> > --- > OvmfPkg/Virtio10Dxe/Virtio10.c | 121 +++++++++++++++++++- > 1 file changed, 119 insertions(+), 2 deletions(-) > > diff --git a/OvmfPkg/Virtio10Dxe/Virtio10.c b/OvmfPkg/Virtio10Dxe/Virtio10.c > index 43dcfd7cb2a4..413ffa06cf35 100644 > --- a/OvmfPkg/Virtio10Dxe/Virtio10.c > +++ b/OvmfPkg/Virtio10Dxe/Virtio10.c > @@ -2,6 +2,7 @@ > A non-transitional driver for VirtIo 1.0 PCI devices. > > Copyright (C) 2016, Red Hat, Inc. > + 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 > @@ -15,6 +16,7 @@ > #include <IndustryStandard/Pci.h> > #include <IndustryStandard/Virtio.h> > #include <Protocol/PciIo.h> > +#include <Protocol/PciRootBridgeIo.h> > #include <Protocol/VirtioDevice.h> > #include <Library/BaseMemoryLib.h> > #include <Library/DebugLib.h> > @@ -772,6 +774,117 @@ Virtio10ReadDevice ( > return Status; > } > > +STATIC > +EFI_STATUS > +EFIAPI > +Virtio10AllocateSharedPages ( > + IN VIRTIO_DEVICE_PROTOCOL *This, > + IN UINTN Pages, > + IN OUT VOID **HostAddress > + ) > +{ > + VIRTIO_1_0_DEV *Dev; > + EFI_STATUS Status; > + > + Dev = VIRTIO_1_0_FROM_VIRTIO_DEVICE (This); > + > + Status = Dev->PciIo->AllocateBuffer ( > + Dev->PciIo, > + AllocateAnyPages, > + EfiBootServicesData, > + Pages, > + HostAddress, > + EFI_PCI_ATTRIBUTE_MEMORY_CACHED > + ); > + return Status; > +} > + > +STATIC > +VOID > +EFIAPI > +Virtio10FreeSharedPages ( > + IN VIRTIO_DEVICE_PROTOCOL *This, > + IN UINTN Pages, > + IN VOID *HostAddress > + ) > +{ > + VIRTIO_1_0_DEV *Dev; > + > + Dev = VIRTIO_1_0_FROM_VIRTIO_DEVICE (This); > + > + Dev->PciIo->FreeBuffer ( > + Dev->PciIo, > + Pages, > + HostAddress > + ); > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +Virtio10MapSharedBuffer ( > + 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 > + ) > +{ > + EFI_STATUS Status; > + VIRTIO_1_0_DEV *Dev; > + EFI_PCI_IO_PROTOCOL_OPERATION PciIoOperation; > + > + Dev = VIRTIO_1_0_FROM_VIRTIO_DEVICE (This); > + > + // > + // Map VIRTIO_MAP_OPERATION to EFI_PCI_IO_PROTOCOL_OPERATION > + // > + switch (Operation) { > + case VirtioOperationBusMasterRead: > + PciIoOperation = EfiPciIoOperationBusMasterRead; > + break; > + case VirtioOperationBusMasterWrite: > + PciIoOperation = EfiPciIoOperationBusMasterWrite; > + break; > + case VirtioOperationBusMasterCommonBuffer: > + PciIoOperation = EfiPciIoOperationBusMasterCommonBuffer; > + break; > + default: > + return EFI_INVALID_PARAMETER; > + } > + > + Status = Dev->PciIo->Map ( > + Dev->PciIo, > + PciIoOperation, > + HostAddress, > + NumberOfBytes, > + DeviceAddress, > + Mapping > + ); > + return Status; > +} > + > +STATIC > +EFI_STATUS > +EFIAPI > +Virtio10UnmapSharedBuffer ( > + IN VIRTIO_DEVICE_PROTOCOL *This, > + IN VOID *Mapping > + ) > +{ > + EFI_STATUS Status; > + VIRTIO_1_0_DEV *Dev; > + > + Dev = VIRTIO_1_0_FROM_VIRTIO_DEVICE (This); > + > + Status = Dev->PciIo->Unmap ( > + Dev->PciIo, > + Mapping > + ); > + > + return Status; > +} > > STATIC CONST VIRTIO_DEVICE_PROTOCOL mVirtIoTemplate = { > VIRTIO_SPEC_REVISION (1, 0, 0), > @@ -788,7 +901,11 @@ STATIC CONST VIRTIO_DEVICE_PROTOCOL mVirtIoTemplate = { > Virtio10GetDeviceStatus, > Virtio10SetDeviceStatus, > Virtio10WriteDevice, > - Virtio10ReadDevice > + Virtio10ReadDevice, > + Virtio10AllocateSharedPages, > + Virtio10FreeSharedPages, > + Virtio10MapSharedBuffer, > + Virtio10UnmapSharedBuffer > }; > > > @@ -906,7 +1023,7 @@ Virtio10BindingStart ( > goto ClosePciIo; > } > > - SetAttributes = EFI_PCI_IO_ATTRIBUTE_BUS_MASTER; > + SetAttributes = EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE; > UpdateAttributes (&Device->CommonConfig, &SetAttributes); > UpdateAttributes (&Device->NotifyConfig, &SetAttributes); > UpdateAttributes (&Device->SpecificConfig, &SetAttributes); >
(1) This patch is almost complete in my opinion, except the last hunk is incorrect -- I think it may be due to an incorrect conflict resolution during rebase. Namely, we shouldn't assign "SetAttributes" the value EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE but the value (EFI_PCI_IO_ATTRIBUTE_BUS_MASTER | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) IOW, adding EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE is good, but EFI_PCI_IO_ATTRIBUTE_BUS_MASTER should be preserved too. Please refer to the following sentence in my v1 review <[email protected]">http://mid.mail-archive.com/[email protected]>, point (5): "(And then do point (4) on top of that, separately.)" Thank you! Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
