[PATCH] x86/traps: Trim includes

Andrew Cooper posted 1 patch 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20250527150911.59744-1-andrew.cooper3@citrix.com
xen/arch/x86/traps.c | 25 -------------------------
1 file changed, 25 deletions(-)
[PATCH] x86/traps: Trim includes
Posted by Andrew Cooper 5 months ago
None of these are used by traps.c today.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

I'm experimenting with include-what-you-use but it's not the most
ergonomic of tools to use.

xen.git/xen$ wc -l arch/x86/traps-{before,after}.i
  29647 arch/x86/traps-before.i
  25355 arch/x86/traps-after.i
---
 xen/arch/x86/traps.c | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index f9e17e015947..092c7e419748 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -17,47 +17,25 @@
 #include <xen/console.h>
 #include <xen/delay.h>
 #include <xen/domain_page.h>
-#include <xen/err.h>
-#include <xen/errno.h>
-#include <xen/event.h>
 #include <xen/guest_access.h>
-#include <xen/hypercall.h>
 #include <xen/init.h>
-#include <xen/iocap.h>
-#include <xen/irq.h>
-#include <xen/kexec.h>
-#include <xen/lib.h>
-#include <xen/livepatch.h>
 #include <xen/mm.h>
 #include <xen/paging.h>
 #include <xen/param.h>
 #include <xen/perfc.h>
 #include <xen/sched.h>
-#include <xen/shutdown.h>
 #include <xen/softirq.h>
-#include <xen/spinlock.h>
-#include <xen/symbols.h>
 #include <xen/trace.h>
-#include <xen/virtual_region.h>
 #include <xen/watchdog.h>
 
-#include <xsm/xsm.h>
-
 #include <asm/apic.h>
-#include <asm/atomic.h>
-#include <asm/cpuid.h>
 #include <asm/debugreg.h>
 #include <asm/desc.h>
 #include <asm/flushtlb.h>
 #include <asm/gdbsx.h>
-#include <asm/hpet.h>
-#include <asm/hvm/vpt.h>
 #include <asm/i387.h>
-#include <asm/idt.h>
 #include <asm/io.h>
 #include <asm/irq-vectors.h>
-#include <asm/mc146818rtc.h>
-#include <asm/mce.h>
 #include <asm/msr.h>
 #include <asm/nmi.h>
 #include <asm/pv/mm.h>
@@ -70,10 +48,7 @@
 #include <asm/system.h>
 #include <asm/traps.h>
 #include <asm/uaccess.h>
-#include <asm/vpmu.h>
-#include <asm/x86_emulate.h>
 #include <asm/xenoprof.h>
-#include <asm/xstate.h>
 
 /*
  * opt_nmi: one of 'ignore', 'dom0', or 'fatal'.
-- 
2.34.1


Re: [PATCH] x86/traps: Trim includes
Posted by Roger Pau Monné 5 months ago
On Tue, May 27, 2025 at 04:09:11PM +0100, Andrew Cooper wrote:
> None of these are used by traps.c today.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> 
> I'm experimenting with include-what-you-use but it's not the most
> ergonomic of tools to use.

Does include-what-you-use take into account #ifdef sections?  I'm
wondering whether non-default Kconfig could require extra headers that
are not accounted for the tool?

Thanks, Roger.

Re: [PATCH] x86/traps: Trim includes
Posted by Andrew Cooper 5 months ago
On 28/05/2025 1:43 pm, Roger Pau Monné wrote:
> On Tue, May 27, 2025 at 04:09:11PM +0100, Andrew Cooper wrote:
>> None of these are used by traps.c today.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks.

>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>
>> I'm experimenting with include-what-you-use but it's not the most
>> ergonomic of tools to use.
> Does include-what-you-use take into account #ifdef sections?  I'm
> wondering whether non-default Kconfig could require extra headers that
> are not accounted for the tool?

Testing suggests that it does a normal preprocessor pass first.

Given:

    #include <xen/bitops.h>

    #if 0
    void *ptr = NULL;
    #endif

the report suggests:

arch/x86/iwyu.c should add these lines:

arch/x86/iwyu.c should remove these lines:
- #include <xen/bitops.h>  // lines 1-1

The full include-list for arch/x86/iwyu.c:
---


and does not make the recommendation to include <xen/types.h> to get NULL.


For traps.c,

grep -e '#ifdef' -e '#.*defined(' -e "IS_ENABLED" arch/x86/traps.c
#ifdef NDEBUG
#ifdef CONFIG_PV32
#ifdef CONFIG_HVM
#ifdef CONFIG_XEN_SHSTK
#if !defined(CONFIG_FRAME_POINTER)
    if ( IS_ENABLED(CONFIG_XEN_SHSTK) )
    if ( IS_ENABLED(CONFIG_DEBUG) && print )
#ifdef CONFIG_PV
#ifdef CONFIG_PV
#ifdef CONFIG_PV
#ifdef CONFIG_PV
        if ( IS_ENABLED(CONFIG_PV) && ret == EXCRET_fault_fixed )
#ifdef CONFIG_PV
#ifdef CONFIG_PV
#ifdef CONFIG_PV
#ifdef CONFIG_DEBUG

and each of HVM, PV and PV32 and XEN_SHSTK are active in the analysis I did.

NDEBUG, CONFIG_DEBUG and CONFIG_FRAME_POINTER aren't doing anything
interesting here.

So I think this patch for traps.c is accurate.

setup.c is far less certain, and I'll need to do some more analysis
before committing that.

~Andrew