[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 1/5] IOMMU: iommu_intremap is x86-only

Hi Jan,

On 02/03/2020 10:57, Jan Beulich wrote:
On 02.03.2020 11:44, Julien Grall wrote:

On 02/03/2020 10:07, Jan Beulich wrote:
On 28.02.2020 21:16, Andrew Cooper wrote:
On 28/02/2020 12:26, Jan Beulich wrote:
Provide a #define for other cases; it didn't seem worthwhile to me to
introduce an IOMMU_INTREMAP Kconfig option at this point.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -1299,6 +1299,8 @@ boolean (e.g. `iommu=no`) can override t
       generation of IOMMUs only supported DMA remapping, and Interrupt 
       appeared in the second generation.
+ This option is not valid on Arm.

The longevity of this comment would be greater if it were phrased as "is
only valid on x86", especially given the RFC RISCV series on list.

How do we know how intremap is going to work on future ports?

We don't  know. But, for a reviewer, it is going to be much easier to
notice a command line option is going to be used as the patch would
modify a caller.

I'm struggling with understanding this (not seeing the connection
between "command line option" and "caller"), but ...

"caller" might have been the wrong word here. Let me expand it. The patch you sent contains an #ifdef CONFIG_X86 protecting the declaration of iommu_intremap:

+#ifdef CONFIG_X86
 extern enum __packed iommu_intremap {
     * In order to allow traditional boolean uses of the iommu_intremap
* variable, the "off" value has to come first (yielding a value of zero).
-#ifdef CONFIG_X86
 } iommu_intremap;
+# define iommu_intremap false

Someone wanted to use iommu_intremap (and by extent the command line option) in a new arch would have to modify the declaration for it to work. A commit message would also likely to contain "Implement the command line option ...". So a reviewer can spot the change and ask to update the documentation if this wasn't yet done.

At the inverse, if the new arch is not using iommu_intremap then there will be no modification in the code. Therefore, it may be more difficult for a reviewer to notice that the documentation needs to be updated.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.