Split the existing asm/microcode.h in half, keeping the per-cpu cpu_sig
available to external users, and moving everything else into private.h

Take the opportunity to trim and clean up the include lists for all 3 source
files, all of which include rather more than necessary.

No functional change.

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

Inclusion of asm/flushtlb.h in isolation was broken by c/s 80943aa40e, and the
commit message even states this breakage.  I'm surprised it got accepted.

Either this needs fixing, or the 23(!) other files including asm/flushtlb.h
should be adjusted.  Personally I don't think it is reasonable to require
including xen/mm.h just to get at tlb flushing functionality, but I also can't
spot an obvious way to untangle the dependencies (hence the TODO).
---
 xen/arch/x86/Makefile                              |  4 +--
 xen/arch/x86/microcode/Makefile                    |  3 ++
 xen/arch/x86/{microcode_amd.c => microcode/amd.c}  | 12 ++++----
 xen/arch/x86/{microcode.c => microcode/core.c}     | 15 +++-------
 .../x86/{microcode_intel.c => microcode/intel.c}   |  9 ++----
 .../microcode.h => arch/x86/microcode/private.h}   | 19 ++++---------
 xen/include/asm-x86/microcode.h                    | 32 ----------------------
 7 files changed, 22 insertions(+), 72 deletions(-)
 create mode 100644 xen/arch/x86/microcode/Makefile
 rename xen/arch/x86/{microcode_amd.c => microcode/amd.c} (99%)
 rename xen/arch/x86/{microcode.c => microcode/core.c} (99%)
 rename xen/arch/x86/{microcode_intel.c => microcode/intel.c} (98%)
 copy xen/{include/asm-x86/microcode.h => arch/x86/microcode/private.h} (79%)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index ed709e2373..fa87520c66 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -43,9 +43,7 @@ obj-$(CONFIG_INDIRECT_THUNK) += indirect-thunk.o
 obj-y += ioport_emulate.o
 obj-y += irq.o
 obj-$(CONFIG_KEXEC) += machine_kexec.o
-obj-y += microcode_amd.o
-obj-y += microcode_intel.o
-obj-y += microcode.o
+obj-y += microcode/
 obj-y += mm.o x86_64/mm.o
 obj-$(CONFIG_HVM) += monitor.o
 obj-y += mpparse.o
diff --git a/xen/arch/x86/microcode/Makefile b/xen/arch/x86/microcode/Makefile
new file mode 100644
index 0000000000..aae235245b
--- /dev/null
+++ b/xen/arch/x86/microcode/Makefile
@@ -0,0 +1,3 @@
+obj-y += amd.o
+obj-y += core.o
+obj-y += intel.o
diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode/amd.c
similarity index 99%
rename from xen/arch/x86/microcode_amd.c
rename to xen/arch/x86/microcode/amd.c
index bc7459416c..9028889813 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode/amd.c
@@ -16,16 +16,14 @@
 
 #include <xen/err.h>
 #include <xen/init.h>
-#include <xen/kernel.h>
-#include <xen/lib.h>
-#include <xen/sched.h>
-#include <xen/smp.h>
-#include <xen/spinlock.h>
+#include <xen/mm.h> /* TODO: Fix asm/tlbflush.h breakage */
 
+#include <asm/hvm/svm/svm.h>
 #include <asm/msr.h>
 #include <asm/processor.h>
-#include <asm/microcode.h>
-#include <asm/hvm/svm/svm.h>
+#include <asm/system.h>
+
+#include "private.h"
 
 #define pr_debug(x...) ((void)0)
 
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode/core.c
similarity index 99%
rename from xen/arch/x86/microcode.c
rename to xen/arch/x86/microcode/core.c
index 6907b312cf..e99f4ab06c 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode/core.c
@@ -22,29 +22,22 @@
  */
 
 #include <xen/cpu.h>
+#include <xen/earlycpio.h>
 #include <xen/err.h>
+#include <xen/guest_access.h>
 #include <xen/init.h>
-#include <xen/kernel.h>
-#include <xen/lib.h>
-#include <xen/notifier.h>
 #include <xen/param.h>
-#include <xen/sched.h>
-#include <xen/smp.h>
-#include <xen/softirq.h>
 #include <xen/spinlock.h>
 #include <xen/stop_machine.h>
-#include <xen/tasklet.h>
-#include <xen/guest_access.h>
-#include <xen/earlycpio.h>
 #include <xen/watchdog.h>
 
 #include <asm/apic.h>
 #include <asm/delay.h>
-#include <asm/msr.h>
 #include <asm/nmi.h>
 #include <asm/processor.h>
 #include <asm/setup.h>
-#include <asm/microcode.h>
+
+#include "private.h"
 
 /*
  * Before performing a late microcode update on any thread, we
diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode/intel.c
similarity index 98%
rename from xen/arch/x86/microcode_intel.c
rename to xen/arch/x86/microcode/intel.c
index 91b7d473f7..90fb006c94 100644
--- a/xen/arch/x86/microcode_intel.c
+++ b/xen/arch/x86/microcode/intel.c
@@ -23,15 +23,12 @@
 
 #include <xen/err.h>
 #include <xen/init.h>
-#include <xen/kernel.h>
-#include <xen/lib.h>
-#include <xen/sched.h>
-#include <xen/smp.h>
-#include <xen/spinlock.h>
 
 #include <asm/msr.h>
 #include <asm/processor.h>
-#include <asm/microcode.h>
+#include <asm/system.h>
+
+#include "private.h"
 
 #define pr_debug(x...) ((void)0)
 
diff --git a/xen/include/asm-x86/microcode.h b/xen/arch/x86/microcode/private.h
similarity index 79%
copy from xen/include/asm-x86/microcode.h
copy to xen/arch/x86/microcode/private.h
index 7d5a1f8e8a..97c7405dad 100644
--- a/xen/include/asm-x86/microcode.h
+++ b/xen/arch/x86/microcode/private.h
@@ -1,7 +1,9 @@
-#ifndef ASM_X86__MICROCODE_H
-#define ASM_X86__MICROCODE_H
+#ifndef ASM_X86_MICROCODE_PRIVATE_H
+#define ASM_X86_MICROCODE_PRIVATE_H
 
-#include <xen/percpu.h>
+#include <xen/types.h>
+
+#include <asm/microcode.h>
 
 enum microcode_match_result {
     OLD_UCODE, /* signature matched, but revision id is older or equal */
@@ -9,8 +11,6 @@ enum microcode_match_result {
     MIS_UCODE, /* signature mismatched */
 };
 
-struct cpu_signature;
-
 struct microcode_patch {
     union {
         struct microcode_intel *mc_intel;
@@ -32,15 +32,8 @@ struct microcode_ops {
         const struct microcode_patch *new, const struct microcode_patch *old);
 };
 
-struct cpu_signature {
-    unsigned int sig;
-    unsigned int pf;
-    unsigned int rev;
-};
-
-DECLARE_PER_CPU(struct cpu_signature, cpu_sig);
 extern const struct microcode_ops *microcode_ops;
 
 void microcode_free_patch(struct microcode_patch *patch);
 
-#endif /* ASM_X86__MICROCODE_H */
+#endif /* ASM_X86_MICROCODE_PRIVATE_H */
diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h
index 7d5a1f8e8a..9b6ff7db08 100644
--- a/xen/include/asm-x86/microcode.h
+++ b/xen/include/asm-x86/microcode.h
@@ -3,35 +3,6 @@
 
 #include <xen/percpu.h>
 
-enum microcode_match_result {
-    OLD_UCODE, /* signature matched, but revision id is older or equal */
-    NEW_UCODE, /* signature matched, but revision id is newer */
-    MIS_UCODE, /* signature mismatched */
-};
-
-struct cpu_signature;
-
-struct microcode_patch {
-    union {
-        struct microcode_intel *mc_intel;
-        struct microcode_amd *mc_amd;
-        void *mc;
-    };
-};
-
-struct microcode_ops {
-    struct microcode_patch *(*cpu_request_microcode)(const void *buf,
-                                                     size_t size);
-    int (*collect_cpu_info)(struct cpu_signature *csig);
-    int (*apply_microcode)(const struct microcode_patch *patch);
-    int (*start_update)(void);
-    void (*end_update_percpu)(void);
-    void (*free_patch)(void *mc);
-    bool (*match_cpu)(const struct microcode_patch *patch);
-    enum microcode_match_result (*compare_patch)(
-        const struct microcode_patch *new, const struct microcode_patch *old);
-};
-
 struct cpu_signature {
     unsigned int sig;
     unsigned int pf;
@@ -39,8 +10,5 @@ struct cpu_signature {
 };
 
 DECLARE_PER_CPU(struct cpu_signature, cpu_sig);
-extern const struct microcode_ops *microcode_ops;
-
-void microcode_free_patch(struct microcode_patch *patch);
 
 #endif /* ASM_X86__MICROCODE_H */
-- 
2.11.0


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

On 18/03/2020 21:05, Andrew Cooper wrote:
> Split the existing asm/microcode.h in half, keeping the per-cpu cpu_sig
> available to external users, and moving everything else into private.h
>
> Take the opportunity to trim and clean up the include lists for all 3 source
> files, all of which include rather more than necessary.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wl@xen.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
>
> Inclusion of asm/flushtlb.h in isolation was broken by c/s 80943aa40e, and the
> commit message even states this breakage.  I'm surprised it got accepted.
>
> Either this needs fixing, or the 23(!) other files including asm/flushtlb.h
> should be adjusted.  Personally I don't think it is reasonable to require
> including xen/mm.h just to get at tlb flushing functionality, but I also can't
> spot an obvious way to untangle the dependencies (hence the TODO).

Actually, I've found that microcode_free_patch() has no external callers.

I've folded the following delta in, to avoid moving a useless function
declaration

diff --git a/xen/arch/x86/microcode/core.c b/xen/arch/x86/microcode/core.c
index e99f4ab06c..19e1d4b221 100644
--- a/xen/arch/x86/microcode/core.c
+++ b/xen/arch/x86/microcode/core.c
@@ -243,7 +243,7 @@ static struct microcode_patch *parse_blob(const char
*buf, size_t len)
     return NULL;
 }
 
-void microcode_free_patch(struct microcode_patch *microcode_patch)
+static void microcode_free_patch(struct microcode_patch *microcode_patch)
 {
     microcode_ops->free_patch(microcode_patch->mc);
     xfree(microcode_patch);
diff --git a/xen/arch/x86/microcode/private.h
b/xen/arch/x86/microcode/private.h
index 97c7405dad..2e3be79eaf 100644
--- a/xen/arch/x86/microcode/private.h
+++ b/xen/arch/x86/microcode/private.h
@@ -34,6 +34,4 @@ struct microcode_ops {
 
 extern const struct microcode_ops *microcode_ops;
 
-void microcode_free_patch(struct microcode_patch *patch);
-
 #endif /* ASM_X86_MICROCODE_PRIVATE_H */


Alternatively, I might consider pulling this and the similar change to
early_microcode_update_cpu() into an earlier patch, to separate the
static-ing of functions from the general moving of code/declarations.

Thoughts?

~Andrew

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

On 18.03.2020 22:42, Andrew Cooper wrote:
> On 18/03/2020 21:05, Andrew Cooper wrote:
>> Split the existing asm/microcode.h in half, keeping the per-cpu cpu_sig
>> available to external users, and moving everything else into private.h
>>
>> Take the opportunity to trim and clean up the include lists for all 3 source
>> files, all of which include rather more than necessary.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Wei Liu <wl@xen.org>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>
>> Inclusion of asm/flushtlb.h in isolation was broken by c/s 80943aa40e, and the
>> commit message even states this breakage.  I'm surprised it got accepted.
>>
>> Either this needs fixing, or the 23(!) other files including asm/flushtlb.h
>> should be adjusted.  Personally I don't think it is reasonable to require
>> including xen/mm.h just to get at tlb flushing functionality, but I also can't
>> spot an obvious way to untangle the dependencies (hence the TODO).
> 
> Actually, I've found that microcode_free_patch() has no external callers.
> 
> I've folded the following delta in, to avoid moving a useless function
> declaration
> 
> diff --git a/xen/arch/x86/microcode/core.c b/xen/arch/x86/microcode/core.c
> index e99f4ab06c..19e1d4b221 100644
> --- a/xen/arch/x86/microcode/core.c
> +++ b/xen/arch/x86/microcode/core.c
> @@ -243,7 +243,7 @@ static struct microcode_patch *parse_blob(const char
> *buf, size_t len)
>      return NULL;
>  }
>  
> -void microcode_free_patch(struct microcode_patch *microcode_patch)
> +static void microcode_free_patch(struct microcode_patch *microcode_patch)
>  {
>      microcode_ops->free_patch(microcode_patch->mc);
>      xfree(microcode_patch);
> diff --git a/xen/arch/x86/microcode/private.h
> b/xen/arch/x86/microcode/private.h
> index 97c7405dad..2e3be79eaf 100644
> --- a/xen/arch/x86/microcode/private.h
> +++ b/xen/arch/x86/microcode/private.h
> @@ -34,6 +34,4 @@ struct microcode_ops {
>  
>  extern const struct microcode_ops *microcode_ops;
>  
> -void microcode_free_patch(struct microcode_patch *patch);
> -
>  #endif /* ASM_X86_MICROCODE_PRIVATE_H */
> 
> 
> Alternatively, I might consider pulling this and the similar change to
> early_microcode_update_cpu() into an earlier patch, to separate the
> static-ing of functions from the general moving of code/declarations.
> 
> Thoughts?

Either way is fine by me, and can have my ack right away.

Jan

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

On 18.03.2020 22:05, Andrew Cooper wrote:
> Split the existing asm/microcode.h in half, keeping the per-cpu cpu_sig
> available to external users, and moving everything else into private.h
> 
> Take the opportunity to trim and clean up the include lists for all 3 source
> files, all of which include rather more than necessary.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
albeit preferably with ...

> ---
>  xen/arch/x86/Makefile                              |  4 +--
>  xen/arch/x86/microcode/Makefile                    |  3 ++
>  xen/arch/x86/{microcode_amd.c => microcode/amd.c}  | 12 ++++----
>  xen/arch/x86/{microcode.c => microcode/core.c}     | 15 +++-------
>  .../x86/{microcode_intel.c => microcode/intel.c}   |  9 ++----
>  .../microcode.h => arch/x86/microcode/private.h}   | 19 ++++---------

... these going into xen/arch/x86/cpu/microcode/. Thoughts?

Jan

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

On 19/03/2020 09:21, Jan Beulich wrote:
> On 18.03.2020 22:05, Andrew Cooper wrote:
>> Split the existing asm/microcode.h in half, keeping the per-cpu cpu_sig
>> available to external users, and moving everything else into private.h
>>
>> Take the opportunity to trim and clean up the include lists for all 3 source
>> files, all of which include rather more than necessary.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> albeit preferably with ...
>
>> ---
>>  xen/arch/x86/Makefile                              |  4 +--
>>  xen/arch/x86/microcode/Makefile                    |  3 ++
>>  xen/arch/x86/{microcode_amd.c => microcode/amd.c}  | 12 ++++----
>>  xen/arch/x86/{microcode.c => microcode/core.c}     | 15 +++-------
>>  .../x86/{microcode_intel.c => microcode/intel.c}   |  9 ++----
>>  .../microcode.h => arch/x86/microcode/private.h}   | 19 ++++---------
> ... these going into xen/arch/x86/cpu/microcode/. Thoughts?

TBH, I've always found the cpu/ directory redundant.  Everything in
arch/x86 is part of the CPU, and these days, even drivers/passthrough is
part of the CPU.

I'm happy to put it wherever makes sense, so long as there is a clear
understanding of why.

~Andrew

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

On 19.03.2020 10:52, Andrew Cooper wrote:
> On 19/03/2020 09:21, Jan Beulich wrote:
>> On 18.03.2020 22:05, Andrew Cooper wrote:
>>> Split the existing asm/microcode.h in half, keeping the per-cpu cpu_sig
>>> available to external users, and moving everything else into private.h
>>>
>>> Take the opportunity to trim and clean up the include lists for all 3 source
>>> files, all of which include rather more than necessary.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> albeit preferably with ...
>>
>>> ---
>>>  xen/arch/x86/Makefile                              |  4 +--
>>>  xen/arch/x86/microcode/Makefile                    |  3 ++
>>>  xen/arch/x86/{microcode_amd.c => microcode/amd.c}  | 12 ++++----
>>>  xen/arch/x86/{microcode.c => microcode/core.c}     | 15 +++-------
>>>  .../x86/{microcode_intel.c => microcode/intel.c}   |  9 ++----
>>>  .../microcode.h => arch/x86/microcode/private.h}   | 19 ++++---------
>> ... these going into xen/arch/x86/cpu/microcode/. Thoughts?
> 
> TBH, I've always found the cpu/ directory redundant.  Everything in
> arch/x86 is part of the CPU, and these days, even drivers/passthrough is
> part of the CPU.

I'm surprised of you saying so - certainly e.g. memory management
stuff also interfaces with the CPU, but is imo still helpful to be
separated. Likewise while IOMMU stuff may today be part of the
CPU package, it's still not core CPU functionality imo.

> I'm happy to put it wherever makes sense, so long as there is a clear
> understanding of why.

The boundaries are always going to be fuzzy, I think. As said, I'd
prefer the alternative place, but I'm not going to insist.

Jan

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

On 19/03/2020 09:59, Jan Beulich wrote:
> On 19.03.2020 10:52, Andrew Cooper wrote:
>> On 19/03/2020 09:21, Jan Beulich wrote:
>>> On 18.03.2020 22:05, Andrew Cooper wrote:
>>>> Split the existing asm/microcode.h in half, keeping the per-cpu cpu_sig
>>>> available to external users, and moving everything else into private.h
>>>>
>>>> Take the opportunity to trim and clean up the include lists for all 3 source
>>>> files, all of which include rather more than necessary.
>>>>
>>>> No functional change.
>>>>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>> albeit preferably with ...
>>>
>>>> ---
>>>>  xen/arch/x86/Makefile                              |  4 +--
>>>>  xen/arch/x86/microcode/Makefile                    |  3 ++
>>>>  xen/arch/x86/{microcode_amd.c => microcode/amd.c}  | 12 ++++----
>>>>  xen/arch/x86/{microcode.c => microcode/core.c}     | 15 +++-------
>>>>  .../x86/{microcode_intel.c => microcode/intel.c}   |  9 ++----
>>>>  .../microcode.h => arch/x86/microcode/private.h}   | 19 ++++---------
>>> ... these going into xen/arch/x86/cpu/microcode/. Thoughts?
>> TBH, I've always found the cpu/ directory redundant.  Everything in
>> arch/x86 is part of the CPU, and these days, even drivers/passthrough is
>> part of the CPU.
> I'm surprised of you saying so - certainly e.g. memory management
> stuff also interfaces with the CPU, but is imo still helpful to be
> separated.

I can see an argument for things like domain.c not living in cpu/, but
where do we draw the line?

Should traps.c be considered cpu/ or not?  What about FPU handling?

> Likewise while IOMMU stuff may today be part of the
> CPU package, it's still not core CPU functionality imo.

Sure, for small systems, but considering it is effectively mandatory for
a >255 cpu system, I'd no longer agree.

After all, we know its not safe running an Intel system until you've
turned on every thread's CR4.MCE, even if you don't actually want to use
the thread.

>
>> I'm happy to put it wherever makes sense, so long as there is a clear
>> understanding of why.
> The boundaries are always going to be fuzzy, I think. As said, I'd
> prefer the alternative place, but I'm not going to insist.

All I'm looking for is some kind of clarity on what this boundary might be.

~Andrew

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

On 19.03.2020 11:41, Andrew Cooper wrote:
> On 19/03/2020 09:59, Jan Beulich wrote:
>> On 19.03.2020 10:52, Andrew Cooper wrote:
>>> On 19/03/2020 09:21, Jan Beulich wrote:
>>>> On 18.03.2020 22:05, Andrew Cooper wrote:
>>>>> Split the existing asm/microcode.h in half, keeping the per-cpu cpu_sig
>>>>> available to external users, and moving everything else into private.h
>>>>>
>>>>> Take the opportunity to trim and clean up the include lists for all 3 source
>>>>> files, all of which include rather more than necessary.
>>>>>
>>>>> No functional change.
>>>>>
>>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Acked-by: Jan Beulich <jbeulich@suse.com>
>>>> albeit preferably with ...
>>>>
>>>>> ---
>>>>>  xen/arch/x86/Makefile                              |  4 +--
>>>>>  xen/arch/x86/microcode/Makefile                    |  3 ++
>>>>>  xen/arch/x86/{microcode_amd.c => microcode/amd.c}  | 12 ++++----
>>>>>  xen/arch/x86/{microcode.c => microcode/core.c}     | 15 +++-------
>>>>>  .../x86/{microcode_intel.c => microcode/intel.c}   |  9 ++----
>>>>>  .../microcode.h => arch/x86/microcode/private.h}   | 19 ++++---------
>>>> ... these going into xen/arch/x86/cpu/microcode/. Thoughts?
>>> TBH, I've always found the cpu/ directory redundant.  Everything in
>>> arch/x86 is part of the CPU, and these days, even drivers/passthrough is
>>> part of the CPU.
>> I'm surprised of you saying so - certainly e.g. memory management
>> stuff also interfaces with the CPU, but is imo still helpful to be
>> separated.
> 
> I can see an argument for things like domain.c not living in cpu/, but
> where do we draw the line?
> 
> Should traps.c be considered cpu/ or not?

Perhaps partly here and there.

>  What about FPU handling?

Yes, this would belong under cpu/ imo.

>> Likewise while IOMMU stuff may today be part of the
>> CPU package, it's still not core CPU functionality imo.
> 
> Sure, for small systems, but considering it is effectively mandatory for
> a >255 cpu system, I'd no longer agree.

That still doesn't make the IOMMU part of the core CPU. Nor
is it technically impossible to run >255 CPU systems without
IOMMU, it's just not very efficient interrupt distribution
wise.

> After all, we know its not safe running an Intel system until you've
> turned on every thread's CR4.MCE, even if you don't actually want to use
> the thread.

Well, CR4.MCE and in fact all MCA handling is CPU stuff,
and hence imo validly lives under cpu/mcheck/.

Jan

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

This reduces the complexity of processor.h, particularly the need to include
public/xen.h.  Substitute processor.h includes for microcode.h in some
sources, and add microcode.h includes in others.

Only 4 of the 7 function declarations are actually called externally.
early_microcode_update_cpu() has no callers so make it static, and move the
vendor init declarations to private.h

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/acpi/power.c         | 1 +
 xen/arch/x86/efi/efi-boot.h       | 2 +-
 xen/arch/x86/microcode/core.c     | 2 +-
 xen/arch/x86/microcode/private.h  | 3 +++
 xen/arch/x86/platform_hypercall.c | 1 +
 xen/arch/x86/setup.c              | 1 +
 xen/arch/x86/smpboot.c            | 1 +
 xen/arch/x86/spec_ctrl.c          | 1 -
 xen/include/asm-x86/microcode.h   | 8 ++++++++
 xen/include/asm-x86/processor.h   | 9 ---------
 10 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index b5df00b22c..e3d6eefe65 100644
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -30,6 +30,7 @@
 #include <asm/tboot.h>
 #include <asm/apic.h>
 #include <asm/io_apic.h>
+#include <asm/microcode.h>
 #include <asm/spec_ctrl.h>
 #include <acpi/cpufreq/cpufreq.h>
 
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index bf7b0a61dc..7bfb96875c 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -6,8 +6,8 @@
 #include <xen/vga.h>
 #include <asm/e820.h>
 #include <asm/edd.h>
+#include <asm/microcode.h>
 #include <asm/msr.h>
-#include <asm/processor.h>
 
 static struct file __initdata ucode;
 static multiboot_info_t __initdata mbi = {
diff --git a/xen/arch/x86/microcode/core.c b/xen/arch/x86/microcode/core.c
index e99f4ab06c..e54a5fec51 100644
--- a/xen/arch/x86/microcode/core.c
+++ b/xen/arch/x86/microcode/core.c
@@ -756,7 +756,7 @@ int microcode_update_one(bool start_update)
 }
 
 /* BSP calls this function to parse ucode blob and then apply an update. */
-int __init early_microcode_update_cpu(void)
+static int __init early_microcode_update_cpu(void)
 {
     int rc = 0;
     const void *data = NULL;
diff --git a/xen/arch/x86/microcode/private.h b/xen/arch/x86/microcode/private.h
index 97c7405dad..cdfe637b6d 100644
--- a/xen/arch/x86/microcode/private.h
+++ b/xen/arch/x86/microcode/private.h
@@ -36,4 +36,7 @@ extern const struct microcode_ops *microcode_ops;
 
 void microcode_free_patch(struct microcode_patch *patch);
 
+int microcode_init_intel(void);
+int microcode_init_amd(void);
+
 #endif /* ASM_X86_MICROCODE_PRIVATE_H */
diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 80efb84328..ee2efdd875 100644
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -27,6 +27,7 @@
 #include <public/platform.h>
 #include <acpi/cpufreq/processor_perf.h>
 #include <asm/edd.h>
+#include <asm/microcode.h>
 #include <asm/mtrr.h>
 #include <asm/io_apic.h>
 #include <asm/setup.h>
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index c87040c890..885919d5c3 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -52,6 +52,7 @@
 #include <asm/cpuid.h>
 #include <asm/spec_ctrl.h>
 #include <asm/guest.h>
+#include <asm/microcode.h>
 
 /* opt_nosmp: If true, secondary processors are ignored. */
 static bool __initdata opt_nosmp;
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 0e54bd14f3..09264b02d1 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -39,6 +39,7 @@
 #include <asm/div64.h>
 #include <asm/flushtlb.h>
 #include <asm/guest.h>
+#include <asm/microcode.h>
 #include <asm/msr.h>
 #include <asm/mtrr.h>
 #include <asm/spec_ctrl.h>
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index aed2c6613a..c5d8e587a8 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -24,7 +24,6 @@
 
 #include <asm/microcode.h>
 #include <asm/msr.h>
-#include <asm/processor.h>
 #include <asm/pv/domain.h>
 #include <asm/pv/shim.h>
 #include <asm/setup.h>
diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h
index 9b6ff7db08..89b9aaa02d 100644
--- a/xen/include/asm-x86/microcode.h
+++ b/xen/include/asm-x86/microcode.h
@@ -1,8 +1,11 @@
 #ifndef ASM_X86__MICROCODE_H
 #define ASM_X86__MICROCODE_H
 
+#include <xen/types.h>
 #include <xen/percpu.h>
 
+#include <public/xen.h>
+
 struct cpu_signature {
     unsigned int sig;
     unsigned int pf;
@@ -11,4 +14,9 @@ struct cpu_signature {
 
 DECLARE_PER_CPU(struct cpu_signature, cpu_sig);
 
+void microcode_set_module(unsigned int idx);
+int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len);
+int early_microcode_init(void);
+int microcode_update_one(bool start_update);
+
 #endif /* ASM_X86__MICROCODE_H */
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index b2b19a02cd..fe231c5072 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -9,7 +9,6 @@
 #include <xen/types.h>
 #include <xen/smp.h>
 #include <xen/percpu.h>
-#include <public/xen.h>
 #include <asm/types.h>
 #include <asm/cpufeature.h>
 #include <asm/desc.h>
@@ -579,14 +578,6 @@ void cpuid_hypervisor_leaves(const struct vcpu *v, uint32_t leaf,
 int guest_rdmsr_xen(const struct vcpu *v, uint32_t idx, uint64_t *val);
 int guest_wrmsr_xen(struct vcpu *v, uint32_t idx, uint64_t val);
 
-void microcode_set_module(unsigned int);
-int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len);
-int early_microcode_update_cpu(void);
-int early_microcode_init(void);
-int microcode_update_one(bool start_update);
-int microcode_init_intel(void);
-int microcode_init_amd(void);
-
 static inline uint8_t get_cpu_family(uint32_t raw, uint8_t *model,
                                      uint8_t *stepping)
 {
-- 
2.11.0


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

On 18.03.2020 22:30, Andrew Cooper wrote:
> This reduces the complexity of processor.h, particularly the need to include
> public/xen.h.  Substitute processor.h includes for microcode.h in some
> sources, and add microcode.h includes in others.
> 
> Only 4 of the 7 function declarations are actually called externally.
> early_microcode_update_cpu() has no callers so make it static, and move the
> vendor init declarations to private.h
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>


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