Re: [PATCH v2 06/23] OvmfPkg/Virtio: introduce IOMMU-like member functions to VIRTIO_DEVICE_PROTOCOL

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

On 08/16/2017 09:37 AM, Laszlo Ersek wrote:
> (1) In my previous review (msgid
> <f953ec03-b160-726b-b9ed-5e3122642815@redhat.com>) point (1), I
> suggested the following subject line:
> 
> "OvmfPkg: introduce IOMMU-like member functions to VIRTIO_DEVICE_PROTOCOL"
> 
> This subject was 72 characters long (within the 74 chars limit).
> 
> Your current subject is:
> 
> "OvmfPkg/Virtio: introduce IOMMU-like member functions to
> VIRTIO_DEVICE_PROTOCOL"
> 
> It is too long (79 chars).
> 
> So please drop the "/Virtio" string, as I requested.
> 

Will do

> On 08/14/17 13:36, Brijesh Singh wrote:
>> The patch extends VIRTIO_DEVICE_PROTOCOL to provide the following new
>> member functions:
>>
>> - AllocateSharedPages : allocate a memory region suitable for sharing
>>     between guest and hypervisor (e.g ring buffer).
>>
>> - FreeSharedPages: free the memory allocated using AllocateSharedPages ().
>>
>> - MapSharedBuffer: map a host address to device address suitable to share
>>     with device for bus master operations.
>>
>> - UnmapSharedBuffer: unmap the device address obtained through the
>>     MapSharedBuffer().
> 
> (2) You missed point (20) of my above-referenced v1 review. Again,
> please append the following to the commit message:
> 
> ---------
> We're free to extend the protocol structure without changing the
> protocol GUID, or bumping any protocol version fields (of which we
> currently have none), because VIRTIO_DEVICE_PROTOCOL is internal to edk2
> by design -- see the disclaimers in "VirtioDevice.h".
> ---------
> 

I will add this text in commit message.

[snip]

> 
> (3) s/taken/token/

I think I will have his error in all the place (due to copy/paste). I will go through
each patches and fix them all.

-Brijesh