Re: [PATCH v1 00/14] OvmfPkg/Virtio: Add APIs to map system physical to device address

[Brijesh Singh <brijesh.singh@amd.com>]

On 08/09/2017 09:39 AM, Laszlo Ersek wrote:
> On 08/07/17 13:58, Brijesh Singh wrote:
>> Currently, virtio drivers provides the system physical address to the device.
>> However, some systems may feature an IOMMU that requires the drivers to pass
>> the device addresses to the device - which are then translated by the IOMMU
>> into physical addresses in memory. The patch series introduces new member
>> functions in VIRTIO_DEVICE_PROTOCOL which can be used for mapping a system
>> physical address to device address.
>>
>> The approach that this patch series takes is to maps the system physical
>> address to device address for buffers (including rings, device specifc
>> request and response  pointed by vring descriptor, and any further memory
>> reference by those request and response).
>>
>> Patch 1 - 3:
>>   Defines and implements new member functions to map a system physical address
>>   to device address. The patch implements Laszlo's suggestion [1].
> 
> (1) I guess under [1] you meant the following message:
> 
> http://mid.mail-archive.com/841bec5f-6f6e-8b1f-25ba-0fd37a915b72@redhat.com
> 

Yes, thank you :) I did not realized that I forgot adding the link.


> If you want, you can add that link to the commit message of patch #1.
> 
> (2) But, that's not my main point here. Before I forget, I'd like to
> point out that you missed one of the three virtio protocol
> implementations -- see my point (5.4) in the above-referenced message
> --, namely:
> 
>> - "ArmVirtPkg/VirtioFdtDxe" (via
>>    "OvmfPkg/Library/VirtioMmioDeviceLib") binds virtio-mmio devices,
>>     and offers virtio 0.9.5 semantics.
> 
> So please replicate patch v1 03/14 to
> "OvmfPkg/Library/VirtioMmioDeviceLib". Otherwise, the modified virtio
> device drivers will crash when they are built for ArmVirtPkg and used
> over virtio-mmio transports.
> 

Sure, I will make the necessary changes in VirtioMmioDeviceLib and try
do the build test but I don't have aarch64 platform to verify at the
runtime.

> (3) Starting with your v2, please add a reminder to your blurb -- for
> Ard and myself -- that before merging this, we should regression-test it
> on aarch64.
> 

Will do