Re: [PATCH v6 01/14] iommu: introduce the concept of BFN...

["Jan Beulich" <JBeulich@suse.com>]

>>> On 05.09.18 at 11:13, <Paul.Durrant@citrix.com> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: 05 September 2018 08:12
>> To: Kevin Tian <kevin.tian@intel.com>
>> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>; Julien Grall
>> <julien.grall@arm.com>; Paul Durrant <Paul.Durrant@citrix.com>; Stefano
>> Stabellini <sstabellini@kernel.org>; xen-devel <xen-
>> devel@lists.xenproject.org>
>> Subject: RE: [Xen-devel] [PATCH v6 01/14] iommu: introduce the concept of
>> BFN...
>> 
>> >>> On 05.09.18 at 08:56, <kevin.tian@intel.com> wrote:
>> >>  From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> Sent: Wednesday, September 5, 2018 2:49 PM
>> >>
>> >> >>> On 05.09.18 at 02:42, <kevin.tian@intel.com> wrote:
>> >> >>  From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> >> Sent: Tuesday, September 4, 2018 5:08 PM
>> >> >>
>> >> >> >>> On 04.09.18 at 10:49, <Paul.Durrant@citrix.com> wrote:
>> >> >> >>  -----Original Message-----
>> >> >> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> >> >> Sent: 04 September 2018 09:47
>> >> >> >> To: Kevin Tian <kevin.tian@intel.com>
>> >> >> >> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>;
>> Julien
>> >> >> Grall
>> >> >> >> <julien.grall@arm.com>; Paul Durrant <Paul.Durrant@citrix.com>;
>> >> >> Stefano
>> >> >> >> Stabellini <sstabellini@kernel.org>; xen-devel <xen-
>> >> >> >> devel@lists.xenproject.org>
>> >> >> >> Subject: Re: [Xen-devel] [PATCH v6 01/14] iommu: introduce the
>> >> concept
>> >> >> of
>> >> >> >> BFN...
>> >> >> >>
>> >> >> >> >>> On 04.09.18 at 10:37, <kevin.tian@intel.com> wrote:
>> >> >> >> >>  From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> >> >> >> Sent: Tuesday, September 4, 2018 4:33 PM
>> >> >> >> >> >
>> >> >> >> >> > bus address is commonly used along with physical/virtual
>> >> address,
>> >> >> to
>> >> >> >> >> > represent different views between devices and CPU. From
>> that
>> >> >> angle
>> >> >> >> >> > I think BFN is a clear term in this context. btw it is not
>> necessary
>> >> to
>> >> >> >> >> > differentiate GBFN and MBFN since there is only one BFN view
>> >> per
>> >> >> >> >> > device.
>> >> >> >> >>
>> >> >> >> >> Sure, but you neglect the presence of one or more IOMMUs
>> when
>> >> >> >> >> you say "between devices and CPU". There addresses prior to
>> and
>> >> >> >> >> after IOMMU translation are distinct, and while the one before
>> the
>> >> >> >> >> translation matches the device view, the one after translation
>> does
>> >> >> >> >> not necessarily match the CPU view. Hence there are two "bus"
>> >> >> >> >> frame numbers here - one representing the device view, and
>> the
>> >> >> >> >> other representing the IOMMU (output) view.
>> >> >> >> >>
>> >> >> >> >
>> >> >> >> > I didn't get. the output address from IOMMU is the one sent to
>> >> >> >> > memory controller, same as the one sent from CPU.
>> >> >> >>
>> >> >> >> That's on present x86 systems, but aiui not in the general case. The
>> >> >> >> terminology to be used in Xen should fit the general case though.
>> >> >> >
>> >> >> > So your concern is cascaded IOMMUs?
>> >> >>
>> >> >> Not primarily. My concern are systems with an I/O address space
>> >> >> (behind the IOMMU) distinct from the CPU address space. Iirc at
>> >> >> least Alpha is/was that way.
>> >> >>
>> >> >
>> >> > Then Paul please documents clearly that this bus address refers to
>> >> > the input side of IOMMU. :-)
>> >>
>> >> But when reading code you can't always go back to look at the one
>> >> place where its meaning is documented. Hence my desire for a name
>> >> which properly conveys the meaning.
>> >>
>> >
>> > Then possibly go back to DFN, but take 'D' as DMA instead of device?
>> 
>> How would "DMA" be any better than "bus"? Whose view it is then still
>> is unclear.
>> 
> 
> Personally I think 'bus address' is commonly enough used term for addresses 
> used by devices for DMA. Indeed we have already 'dev_bus_addr' in the grant 
> map and unmap hypercalls. So baddr and bfn seem like ok terms to me. It's 
> also not impossible to rename these later if they prove problematic.

But that's the point - the names are problematic (to me): I permanently
have to remind myself that they do _not_ refer to the addresses as
seen when accessing memory, but the ones going _into_ the IOMMU.
The confusion (on my part) arises every time I see a mixture of gfn, bfn,
and mfn in the same patch, perhaps including some 1:1-ness assumptions
between pairs of them.

Take these two hunks as example (mixing in some pfn as well):

@@ -436,7 +436,7 @@ static int iommu_merge_pages(struct domain *d, unsigned long pt_mfn,
  * {Re, un}mapping super page frames causes re-allocation of io
  * page tables.
  */
-static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn, 
+static int iommu_pde_from_bfn(struct domain *d, unsigned long pfn,
                               unsigned long pt_mfn[])
 {
     u64 *pde, *next_table_vaddr;
@@ -477,11 +477,11 @@ static int iommu_pde_from_gfn(struct domain *d, unsigned long pfn,
              next_table_mfn != 0 )
         {
             int i;
-            unsigned long mfn, gfn;
+            unsigned long mfn, bfn;
             unsigned int page_sz;
 
             page_sz = 1 << (PTE_PER_TABLE_SHIFT * (next_level - 1));
-            gfn =  pfn & ~((1 << (PTE_PER_TABLE_SHIFT * next_level)) - 1);
+            bfn =  pfn & ~((1 << (PTE_PER_TABLE_SHIFT * next_level)) - 1);
             mfn = next_table_mfn;
 
             /* allocate lower level page table */

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel