Re: [Xen-devel] [PATCH v6 10/14] arm/guest_access: Rename vgic_access_guest_memory

[Sergej Proskurin <proskurin@xxxxxxxxxxxxx>]

Hi Julien,


On 07/17/2017 05:43 PM, Julien Grall wrote:
> Hi Sergej,
>
> On 06/07/17 12:50, Sergej Proskurin wrote:
>> This commit renames the function vgic_access_guest_memory to
>> access_guest_memory_by_ipa. As the function name suggests, the functions
>> expects an ipa as argument. Thus, to make the function's purpose more
>
> s/ipa/IPA/
>
>> clearly, we have also renamed the argument gva into ipa. All invocations
>
> The argument is call gpa not gva. gpa stands for "Guest Physical
> Address" which is the name commonly used in Xen. IPA is the ARM naming.
>

Thanks. I have mistyped that one.

> So I am not convinced of the usefulness of this rename.
>

I Agree. As you have suggested to use access_guest_memory_by_ipa as
function name, I just wanted to be consistent. How about renaming the
function to access_guest_memory_by_gpa instead? Then, we would remain
consistent with Xen's naming conventions also on the function level.

>> of this function have been adapted accordingly.
>>
>> Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
>> ---
>> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> Cc: Julien Grall <julien.grall@xxxxxxx>
>> ---
>> v6: We added this patch to our patch series.
>> ---
>>  xen/arch/arm/guestcopy.c           |  8 ++++----
>>  xen/arch/arm/vgic-v3-its.c         | 36
>> ++++++++++++++++++------------------
>>  xen/include/asm-arm/guest_access.h |  4 ++--
>>  3 files changed, 24 insertions(+), 24 deletions(-)
>>
>> diff --git a/xen/arch/arm/guestcopy.c b/xen/arch/arm/guestcopy.c
>> index 938ffe2668..9ea8cb79a4 100644
>> --- a/xen/arch/arm/guestcopy.c
>> +++ b/xen/arch/arm/guestcopy.c
>> @@ -123,11 +123,11 @@ unsigned long raw_copy_from_guest(void *to,
>> const void __user *from, unsigned le
>>   * Temporarily map one physical guest page and copy data to or from it.
>>   * The data to be copied cannot cross a page boundary.
>>   */
>> -int vgic_access_guest_memory(struct domain *d, paddr_t gpa, void *buf,
>> -                             uint32_t size, bool is_write)
>> +int access_guest_memory_by_ipa(struct domain *d, paddr_t ipa, void
>> *buf,
>> +                               uint32_t size, bool is_write)
>>  {
>>      struct page_info *page;
>> -    uint64_t offset = gpa & ~PAGE_MASK;  /* Offset within the mapped
>> page */
>> +    uint64_t offset = ipa & ~PAGE_MASK;  /* Offset within the mapped
>> page */
>>      p2m_type_t p2mt;
>>      void *p;
>>
>> @@ -139,7 +139,7 @@ int vgic_access_guest_memory(struct domain *d,
>> paddr_t gpa, void *buf,
>>          return -EINVAL;
>>      }
>>
>> -    page = get_page_from_gfn(d, paddr_to_pfn(gpa), &p2mt, P2M_ALLOC);
>> +    page = get_page_from_gfn(d, paddr_to_pfn(ipa), &p2mt, P2M_ALLOC);
>>      if ( !page )
>>      {
>>          printk(XENLOG_G_ERR "d%d: vITS: Failed to get table entry\n",
>
> You want to remove any mention of vITS in all the printks.

Thanks,
~Sergej


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel