[edk2] [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses

Laszlo Ersek posted 6 patches 7 years, 3 months ago
Failed in applying to current master (apply log)
OvmfPkg/VirtioGpuDxe/VirtioGpu.h     |  89 ++++++-
OvmfPkg/VirtioGpuDxe/Commands.c      | 263 ++++++++++++++++++--
OvmfPkg/VirtioGpuDxe/DriverBinding.c |   1 -
OvmfPkg/VirtioGpuDxe/Gop.c           |  62 +++--
4 files changed, 370 insertions(+), 45 deletions(-)
[edk2] [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses
Posted by Laszlo Ersek 7 years, 3 months ago
Repo:   https://github.com/lersek/edk2.git
Branch: virtio_gpu_map

This series brings IOMMU support to VirtioGpuDxe. The interesting part
is patch#5.

I formatted the patches with function context, for easier review.

I regression-tested this series in various environments; IA32, X64,
AARCH64. My access to a SEV machine is still in the making, so I
couldn't test the patches on SEV.

Brijesh, if you could test the series for me on SEV, that would be
great. I also plan to test it on SEV, once my access is established.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>

Thanks,
Laszlo

Laszlo Ersek (6):
  OvmfPkg/VirtioGpuDxe: map VRING for bus master common buffer operation
  OvmfPkg/VirtioGpuDxe: map virtio GPU command objects to device
    addresses
  OvmfPkg/VirtioGpuDxe: take EFI_PHYSICAL_ADDRESS in
    ResourceAttachBacking()
  OvmfPkg/VirtioGpuDxe: helpers for backing store
    (de)allocation+(un)mapping
  OvmfPkg/VirtioGpuDxe: map backing store to bus master device address
  OvmfPkg/VirtioGpuDxe: negotiate VIRTIO_F_IOMMU_PLATFORM

 OvmfPkg/VirtioGpuDxe/VirtioGpu.h     |  89 ++++++-
 OvmfPkg/VirtioGpuDxe/Commands.c      | 263 ++++++++++++++++++--
 OvmfPkg/VirtioGpuDxe/DriverBinding.c |   1 -
 OvmfPkg/VirtioGpuDxe/Gop.c           |  62 +++--
 4 files changed, 370 insertions(+), 45 deletions(-)

-- 
2.14.1.3.gb7cf6e02401b

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses
Posted by Laszlo Ersek 7 years, 3 months ago
On 08/28/17 15:24, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: virtio_gpu_map
>
> This series brings IOMMU support to VirtioGpuDxe. The interesting part
> is patch#5.
>
> I formatted the patches with function context, for easier review.
>
> I regression-tested this series in various environments; IA32, X64,
> AARCH64. My access to a SEV machine is still in the making, so I
> couldn't test the patches on SEV.
>
> Brijesh, if you could test the series for me on SEV, that would be
> great. I also plan to test it on SEV, once my access is established.

I have great test results on SEV. Everything works as expected:

- normal display behavior (setup browser, shell commands, scrolling),

- connect, disconnect, reconnect in the UEFI shell, issued from the
  serial console,

- mode changes in the UEFI shell (after reconnect too).

I can see the IOMMU / SEV library debug messages too, and I can
associate them with GPU actions.

I also tested ExitBootServices(). In the log, I see the following six
(+1) consecutive lines (broken up here for easier reading):

>  244888 IoMmuUnmap PlainText 0x7D273000 Crypted 0x7D273000 Pages 0x300 Bytes 0x300000
>  244889 IoMmuDxe:InternalMemEncryptSevSetMemoryEncrypted Set C-bit Cr3 0 Base 7D273000 Length 300000 flush 1

>  244890 IoMmuUnmap PlainText 0x7ED81000 Crypted 0x7ED81000 Pages 0x2 Bytes 0x2000
>  244891 IoMmuDxe:InternalMemEncryptSevSetMemoryEncrypted Set C-bit Cr3 0 Base 7ED81000 Length 2000 flush 1

>  244892 IoMmuUnmap PlainText 0x7F0DA000 Crypted 0x7F0DA000 Pages 0x2 Bytes 0x2000
>  244893 IoMmuDxe:InternalMemEncryptSevSetMemoryEncrypted Set C-bit Cr3 0 Base 7F0DA000 Length 2000 flush 1

>  244894 MpInitChangeApLoopCallback() done!

* The first two lines correspond to unmapping (re-encrypting) the
  backing store of the GOP's current video mode (see VirtioGpuExitBoot()
  in patch #5). Searching the log backwards for the same address
  (0x7D273000), I find it associated with the latest MODE change from
  the UEFI shell (when this backing store was allocated and decrypted):

>  188052 IoMmuAllocateBuffer Address 0x7D273000 Pages 0x300
>  188053 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit Cr3 0 Base 7D273000 Length 300000 flush 1
>  188054 IoMmuDxe:SetMemoryEncDec Spliting 2MB page at 7D273000
>  188055 IoMmuMap PlainText 0x7D273000 Crypted 0x7D273000 Pages 0x300 Bytes 0x300000

* The second two lines correspond to unmapping (re-encrypting) the
  virtio ring for the virtio-gpu device (see VirtioGpuExitBoot() in
  patch #1). Searching the log backwards for the same address
  (0x7ED81000), I find it associated with the latest connecting of the
  virtio-gpu device, which is when the virtio-ring is allocated and
  mapped/decrypted (see VirtioRingMap() in patch #1):

>  158667 InstallProtocolInterface: [VirtioDeviceProtocol] 7F104B20
>  158668 IoMmuAllocateBuffer Address 0x7ED81000 Pages 0x2
>  158669 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit Cr3 0 Base 7ED81000 Length 2000 flush 1
>  158670 IoMmuMap PlainText 0x7ED81000 Crypted 0x7ED81000 Pages 0x2 Bytes 0x2000
>  ...
>  158700 InstallProtocolInterface: [EfiGraphicsOutputProtocol] 7F1546C0
>  158701 VirtioGpuDriverBindingStart: produced GOP while binding VirtIo=7F104B20

* The third pair of lines stands for unmapping the virtio ring of the
  virtio-blk device that I have also configured for the guest. Searching
  the log backwards for the same address (0x7F0DA000), we find

>    7347 IoMmuAllocateBuffer Address 0x7F0DA000 Pages 0x2
>    7348 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit Cr3 0 Base 7F0DA000 Length 2000 flush 1
>    7349 IoMmuMap PlainText 0x7F0DA000 Crypted 0x7F0DA000 Pages 0x2 Bytes 0x2000
>    7350 VirtioBlkInit: LbaSize=0x200[B] NumBlocks=0xC800000[Lba]
>    7351 VirtioBlkInit: FirstAligned=0x0[Lba] PhysBlkSize=0x1[Lba]
>    7352 VirtioBlkInit: OptimalTransferLengthGranularity=0x0[Lba]
>    7353 InstallProtocolInterface: [EfiBlockIoProtocol] 7F0DE310
>    7354 InstallProtocolInterface: [EfiDiskIoProtocol] 7F0DE4A0

* The final line that I quoted ("MpInitChangeApLoopCallback") is just
  another (unrelated) ExitBootServices() handler's message.

I'm going to call these results successful.

Thanks,
Laszlo

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (6):
>   OvmfPkg/VirtioGpuDxe: map VRING for bus master common buffer operation
>   OvmfPkg/VirtioGpuDxe: map virtio GPU command objects to device
>     addresses
>   OvmfPkg/VirtioGpuDxe: take EFI_PHYSICAL_ADDRESS in
>     ResourceAttachBacking()
>   OvmfPkg/VirtioGpuDxe: helpers for backing store
>     (de)allocation+(un)mapping
>   OvmfPkg/VirtioGpuDxe: map backing store to bus master device address
>   OvmfPkg/VirtioGpuDxe: negotiate VIRTIO_F_IOMMU_PLATFORM
>
>  OvmfPkg/VirtioGpuDxe/VirtioGpu.h     |  89 ++++++-
>  OvmfPkg/VirtioGpuDxe/Commands.c      | 263 ++++++++++++++++++--
>  OvmfPkg/VirtioGpuDxe/DriverBinding.c |   1 -
>  OvmfPkg/VirtioGpuDxe/Gop.c           |  62 +++--
>  4 files changed, 370 insertions(+), 45 deletions(-)
>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses
Posted by Brijesh Singh 7 years, 3 months ago

On 08/29/2017 06:02 PM, Laszlo Ersek wrote:

[...]
> 
> * The third pair of lines stands for unmapping the virtio ring of the
>    virtio-blk device that I have also configured for the guest. Searching
>    the log backwards for the same address (0x7F0DA000), we find
> 
>>     7347 IoMmuAllocateBuffer Address 0x7F0DA000 Pages 0x2
>>     7348 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit Cr3 0 Base 7F0DA000 Length 2000 flush 1
>>     7349 IoMmuMap PlainText 0x7F0DA000 Crypted 0x7F0DA000 Pages 0x2 Bytes 0x2000
>>     7350 VirtioBlkInit: LbaSize=0x200[B] NumBlocks=0xC800000[Lba]
>>     7351 VirtioBlkInit: FirstAligned=0x0[Lba] PhysBlkSize=0x1[Lba]
>>     7352 VirtioBlkInit: OptimalTransferLengthGranularity=0x0[Lba]
>>     7353 InstallProtocolInterface: [EfiBlockIoProtocol] 7F0DE310
>>     7354 InstallProtocolInterface: [EfiDiskIoProtocol] 7F0DE4A0
> 
> * The final line that I quoted ("MpInitChangeApLoopCallback") is just
>    another (unrelated) ExitBootServices() handler's message.
> 
> I'm going to call these results successful.
> 
> Thanks,
> Laszlo
> 
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>


Today I tested your branch on SEV guest and it all worked. thank you for the patch

Tested-By: Brijesh Singh <brijesh.singh@amd.com>


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses
Posted by Laszlo Ersek 7 years, 3 months ago
On 08/30/17 17:47, Brijesh Singh wrote:
> 
> 
> On 08/29/2017 06:02 PM, Laszlo Ersek wrote:
> 
> [...]
>>
>> * The third pair of lines stands for unmapping the virtio ring of the
>>    virtio-blk device that I have also configured for the guest. Searching
>>    the log backwards for the same address (0x7F0DA000), we find
>>
>>>     7347 IoMmuAllocateBuffer Address 0x7F0DA000 Pages 0x2
>>>     7348 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit
>>> Cr3 0 Base 7F0DA000 Length 2000 flush 1
>>>     7349 IoMmuMap PlainText 0x7F0DA000 Crypted 0x7F0DA000 Pages 0x2
>>> Bytes 0x2000
>>>     7350 VirtioBlkInit: LbaSize=0x200[B] NumBlocks=0xC800000[Lba]
>>>     7351 VirtioBlkInit: FirstAligned=0x0[Lba] PhysBlkSize=0x1[Lba]
>>>     7352 VirtioBlkInit: OptimalTransferLengthGranularity=0x0[Lba]
>>>     7353 InstallProtocolInterface: [EfiBlockIoProtocol] 7F0DE310
>>>     7354 InstallProtocolInterface: [EfiDiskIoProtocol] 7F0DE4A0
>>
>> * The final line that I quoted ("MpInitChangeApLoopCallback") is just
>>    another (unrelated) ExitBootServices() handler's message.
>>
>> I'm going to call these results successful.
>>
>> Thanks,
>> Laszlo
>>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>>
> 
> 
> Today I tested your branch on SEV guest and it all worked. thank you for
> the patch
> 
> Tested-By: Brijesh Singh <brijesh.singh@amd.com>

Thank you!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH 0/6] OvmfPkg/VirtioGpuDxe: map system memory addresses to device addresses
Posted by Laszlo Ersek 7 years, 3 months ago
On 08/30/17 17:47, Brijesh Singh wrote:
> 
> 
> On 08/29/2017 06:02 PM, Laszlo Ersek wrote:
> 
> [...]
>>
>> * The third pair of lines stands for unmapping the virtio ring of the
>>    virtio-blk device that I have also configured for the guest. Searching
>>    the log backwards for the same address (0x7F0DA000), we find
>>
>>>     7347 IoMmuAllocateBuffer Address 0x7F0DA000 Pages 0x2
>>>     7348 IoMmuDxe:InternalMemEncryptSevSetMemoryDecrypted Clear C-bit
>>> Cr3 0 Base 7F0DA000 Length 2000 flush 1
>>>     7349 IoMmuMap PlainText 0x7F0DA000 Crypted 0x7F0DA000 Pages 0x2
>>> Bytes 0x2000
>>>     7350 VirtioBlkInit: LbaSize=0x200[B] NumBlocks=0xC800000[Lba]
>>>     7351 VirtioBlkInit: FirstAligned=0x0[Lba] PhysBlkSize=0x1[Lba]
>>>     7352 VirtioBlkInit: OptimalTransferLengthGranularity=0x0[Lba]
>>>     7353 InstallProtocolInterface: [EfiBlockIoProtocol] 7F0DE310
>>>     7354 InstallProtocolInterface: [EfiDiskIoProtocol] 7F0DE4A0
>>
>> * The final line that I quoted ("MpInitChangeApLoopCallback") is just
>>    another (unrelated) ExitBootServices() handler's message.
>>
>> I'm going to call these results successful.
>>
>> Thanks,
>> Laszlo
>>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Cc: Brijesh Singh <brijesh.singh@amd.com>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>>>
> 
> 
> Today I tested your branch on SEV guest and it all worked. thank you for
> the patch
> 
> Tested-By: Brijesh Singh <brijesh.singh@amd.com>

Thanks again, pushed as commit range 1afbb85f8736..db52890926b6.

Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel