diff mbox series

[v2,02/15] genpt: Add Documentation/ files

Message ID 2-v2-5c26bde5c22d+58b-iommu_pt_jgg@nvidia.com
State New
Headers show
Series Consolidate iommu page table implementations (AMD) | expand

Commit Message

Jason Gunthorpe May 5, 2025, 2:18 p.m. UTC 
Add some general description and pull in the kdoc comments from the source
file to index most of the useful functions.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 Documentation/driver-api/generic_pt.rst | 105 ++++++++++++++++++++++++
 Documentation/driver-api/index.rst      |   1 +
 2 files changed, 106 insertions(+)
 create mode 100644 Documentation/driver-api/generic_pt.rst

Comments

Bagas Sanjaya May 7, 2025, 2:37 a.m. UTC | #1 
On Mon, May 05, 2025 at 11:18:32AM -0300, Jason Gunthorpe wrote:
> +Since each compilation unit can only access one underlying format at a time,
> +code that is intended to be generic across multiple formats has to compile
> +itself multiple times.
> +
> +In an implementation compilation unit the headers would normally be included as
> +follows::
> +
> +	#include <linux/generic_pt/common.h>
> +	#include "fmt/defs_amdv1.h"
> +	#include "pt_defs.h"
> +	#include "fmt/amdv1.h"
> +	#include "pt_common.h"
> +	#include "pt_iter.h"

What do you mean by compiling generic code multiple times? Including
their headers at multiple places like above?

> +
> +Which will build up all the definitions to operate an AMDv1 page table type.

"This will build up ..."

Thanks.
Jason Gunthorpe May 13, 2025, 6:53 p.m. UTC | #2 
On Wed, May 07, 2025 at 09:37:07AM +0700, Bagas Sanjaya wrote:
> On Mon, May 05, 2025 at 11:18:32AM -0300, Jason Gunthorpe wrote:
> > +Since each compilation unit can only access one underlying format at a time,
> > +code that is intended to be generic across multiple formats has to compile
> > +itself multiple times.
> > +
> > +In an implementation compilation unit the headers would normally be included as
> > +follows::
> > +
> > +	#include <linux/generic_pt/common.h>
> > +	#include "fmt/defs_amdv1.h"
> > +	#include "pt_defs.h"
> > +	#include "fmt/amdv1.h"
> > +	#include "pt_common.h"
> > +	#include "pt_iter.h"
> 
> What do you mean by compiling generic code multiple times? Including
> their headers at multiple places like above?

How about like this:

Instead the function pointers can end up at the higher level API (ie map/unmap,
etc) and the per-format code can be directly inlined into the per-format
compilation unit. For something like iommu each format will be compiled into a
per-format iommu operations kernel module.

For this to work the .c file for each compilation unit will include both the
format headers and the generic code for the implementation. For instance in an
implementation compilation unit the headers would normally be included as
follows::

	#include <linux/generic_pt/common.h>
	#include "fmt/defs_amdv1.h"
	#include "pt_defs.h"
	#include "fmt/amdv1.h"
	#include "pt_common.h"
	#include "pt_iter.h"
	#include "iommut_pt.h"  /* The iommu implementation */

iommu_pt.h includes definitions that will generate the operations functions for
map/unmap/etc using the definitions provided by AMDv1. The resulting module
will have exported symbols named like pt_iommu_amdv1_init().

Thanks,
Jason
diff mbox series

Patch

diff --git a/Documentation/driver-api/generic_pt.rst b/Documentation/driver-api/generic_pt.rst
new file mode 100644
index 00000000000000..4fb506a95be40d
--- /dev/null
+++ b/Documentation/driver-api/generic_pt.rst
@@ -0,0 +1,105 @@ 
+.. SPDX-License-Identifier: GPL-2.0
+
+========================
+Generic Radix Page Table
+========================
+
+.. kernel-doc:: include/linux/generic_pt/common.h
+	:doc: Generic Radix Page Table
+
+.. kernel-doc:: drivers/iommu/generic_pt/pt_defs.h
+	:doc: Generic Page Table Language
+
+-----
+Usage
+-----
+
+Generic PT is structured as a multi-compilation system. Since each format
+provides an API using a common set of names there can be only one format active
+within a compilation unit. This design avoids function pointers around the low
+level API.
+
+Instead the function pointers can end up at the higher level API (ie map/unmap,
+etc) and the per-format code can be directly inlined.
+
+Since each compilation unit can only access one underlying format at a time,
+code that is intended to be generic across multiple formats has to compile
+itself multiple times.
+
+In an implementation compilation unit the headers would normally be included as
+follows::
+
+	#include <linux/generic_pt/common.h>
+	#include "fmt/defs_amdv1.h"
+	#include "pt_defs.h"
+	#include "fmt/amdv1.h"
+	#include "pt_common.h"
+	#include "pt_iter.h"
+
+Which will build up all the definitions to operate an AMDv1 page table type.
+
+Refer to drivers/iommu/generic-pt/fmt/iommu_template.h for an example of how the
+iommu implementation uses multi-compilation to generate per-format ops structs
+pointers.
+
+The format code is written so that the common names arise from #defines to
+distinct format specific names. This is intended to aid debuggability by
+avoiding symbol clashes across all the different formats.
+
+The format uses struct pt_common as the top level struct for the table,
+and each format will have its own struct pt_xxx which embeds it to store
+format-specific information.
+
+The implementation will further wrapper this in its own top level struct, such
+as struct pt_iommu_amdv1.
+
+----------------------------------------------
+Format functions at the struct pt_common level
+----------------------------------------------
+
+.. kernel-doc:: include/linux/generic_pt/common.h
+	:identifiers:
+.. kernel-doc:: drivers/iommu/generic_pt/pt_common.h
+
+-----------------
+Iteration Helpers
+-----------------
+
+.. kernel-doc:: drivers/iommu/generic_pt/pt_iter.h
+
+----------------
+Writing a Format
+----------------
+
+It is best to start from a simple format that is similar to the target. x86_64
+is usually a good reference for something simple, and AMDv1 is something fairly
+complete.
+
+The required inline functions need to be implemented in the format header.
+These should all follow the standard pattern of::
+
+ static inline pt_oaddr_t amdv1pt_entry_oa(const struct pt_state *pts)
+ {
+	[..]
+ }
+ #define pt_entry_oa amdv1pt_entry_oa
+
+Where a uniquely named per-format inline function provides the implementation
+and a define maps it to the generic name. This is intended to make debug symbols
+work better. inline functions should always be used as the prototypes in
+pt_common.h will cause the compiler to validate the function signature to
+prevent errors.
+
+Review pt_fmt_defaults.h to understand some of the optional inlines.
+
+Once the format compiles then it should be run through the generic page table
+kunit test in kunit_generic_pt.h using kunit. For example::
+
+   $ tools/testing/kunit/kunit.py run --build_dir build_kunit_x86_64 --arch x86_64 --kunitconfig ./drivers/iommu/generic_pt/.kunitconfig amdv1_fmt_test.*
+   [...]
+   [11:15:08] Testing complete. Ran 9 tests: passed: 9
+   [11:15:09] Elapsed time: 3.137s total, 0.001s configuring, 2.368s building, 0.311s running
+
+The generic tests are intended to prove out the format functions and give
+clearer failures to speed finding the problems. Once those pass then the entire
+kunit suite should be run.
diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
index 16e2c4ec3c010b..7459f4068d32b0 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -92,6 +92,7 @@  Subsystem-specific APIs
    frame-buffer
    aperture
    generic-counter
+   generic_pt
    gpio/index
    hsi
    hte/index