Re: [Qemu-devel] [PATCH RFC v3 03/14] intel_iommu: renaming gpa to iova where proper

[Tian, Kevin]

> From: Peter Xu [mailto:address@hidden
> Sent: Friday, January 20, 2017 5:24 PM
> 
> On Fri, Jan 20, 2017 at 08:27:31AM +0000, Tian, Kevin wrote:
> > > From: Peter Xu [mailto:address@hidden
> > > Sent: Friday, January 13, 2017 11:06 AM
> > >
> > > There are lots of places in current intel_iommu.c codes that named
> > > "iova" as "gpa". It is really confusing to use a name "gpa" in these
> > > places (which is very easily to be understood as "Guest Physical
> > > Address", while it's not). To make the codes (much) easier to be read, I
> > > decided to do this once and for all.
> > >
> > > No functional change is made. Only literal ones.
> >
> > If looking at VT-d spec (3.2 Domains and Address Translation)
> >
> >     Remapping hardware treats the address in inbound requests as DMA
> >     Address. Depending on the software usage model, the DMA address
> >     space may be the Guest-Physical Address (GPA) space of the virtual
> >     machine to which the device is assigned, or application Virtual Address
> >     (VA) space defined by the PASID assigned to an application, or some
> >     abstract I/O virtual address (IOVA) space defined by software.
> >
> >     For simplicity, this document refers to address in requests-without-
> >     PASID as GPA, and address in requests-with-PASID as Virtual Address
> >     (VA) (or Guest Virtual Address (GVA), if such request is from a device
> >     assigned to a virtual machine). The translated address is referred to as
> >     HPA.
> >
> > It will add more readability if similar comment is added in this file - you
> > can say choosing iova to represent address in requests-without-PASID.
> 
> I agree that the code will be more readable if we can explain all the
> bits in detail.
> 
> However this patch is not adding comments, but to "fix" an existing
> literal problem, which is very misleading to people reading the codes
> for the first time. The places touched in this patch was doing the
> namings incorrectly, so I just corrected them. So even if we want to
> have more comments on explaining the bits, IMHO it'll be nicer to use
> a separate patch, not squashing all the things into a single one.
> 
> If you won't disagree, I'd like to keep this single patch as-it-is, to
> make sure this series can converge soon (I will be sorry since I'll
> extend this series a bit, I hope that won't extend the review process
> too long for it). After that, we can add more documentations if
> needed.
> 

fine with me.

Thanks
Kevin