>>> On 25.04.18 at 11:51, <davidw...@zhaoxin.com> wrote:
> --- a/xen/arch/x86/cpu/intel_cacheinfo.c
> +++ b/xen/arch/x86/cpu/intel_cacheinfo.c
> @@ -103,7 +103,7 @@ int cpuid4_cache_lookup(int index, struct cpuid4_info
> *this_leaf)
> return 0;
> }
>
> -static int find_num_cache_leaves(void)
> +int find_num_cache_leaves(void)
Instead of making this function non-static, please consider re-using
init_intel_cacheinfo(): All you want is skip the CPUID leaf 2 handling,
and you'd better to this by altering the single if() controlling it in that
function than by effectively introducing a clone. If you're concerned
of some other dead code in that function, attached you'll find a patch
deleting at least some of that.
> --- /dev/null
> +++ b/xen/arch/x86/cpu/shanghai.c
> @@ -0,0 +1,90 @@
> +#include <xen/bitops.h>
> +#include <xen/init.h>
> +#include <asm/processor.h>
> +#include "cpu.h"
> +
> +void init_shanghai_cache(struct cpuinfo_x86 *c)
> +{
> + unsigned int i = 0, l1d = 0, l1i = 0, l2 = 0, l3 = 0;
> + struct cpuid4_info leaf;
> + static bool is_initialized = false;
> + static unsigned int cache_leaves = 0;
> +
> + if ( (!is_initialized) && (c->cpuid_level > 0x00000003) )
> + {
If there was a convincing argument that this clone of the original
function was really needed, then you'd need to go through here
and clean up style (various aspects of it are broken, most notably
the mix of space and tab indentation).
> --- a/xen/include/asm-x86/iommu.h
> +++ b/xen/include/asm-x86/iommu.h
> @@ -54,6 +54,7 @@ static inline const struct iommu_ops *iommu_get_ops(void)
> switch ( boot_cpu_data.x86_vendor )
> {
> case X86_VENDOR_INTEL:
> + case X86_VENDOR_SHANGHAI:
> return &intel_iommu_ops;
> case X86_VENDOR_AMD:
> return &amd_iommu_ops;
> @@ -69,6 +70,7 @@ static inline int iommu_hardware_setup(void)
> switch ( boot_cpu_data.x86_vendor )
> {
> case X86_VENDOR_INTEL:
> + case X86_VENDOR_SHANGHAI:
> return intel_vtd_setup();
> case X86_VENDOR_AMD:
> return amd_iov_detect();
There are numerous further occurrences of X86_VENDOR_INTEL throughout
the code base - is it really the case that no single one of them needs similar
amendment?
Jan
x86: remove read code from cpuid4_cache_lookup()
... and make num_cache_leaves local to the only function using it.
Signed-off-by: Jan Beulich <jbeul...@suse.com>
--- unstable.orig/xen/arch/x86/cpu/intel_cacheinfo.c 2017-03-03
14:08:33.000000000 +0100
+++ unstable/xen/arch/x86/cpu/intel_cacheinfo.c 2018-04-30 15:59:54.637217413
+0200
@@ -80,8 +80,6 @@ static const struct _cache_table cache_t
{ 0x00, 0, 0}
};
-unsigned short num_cache_leaves;
-
int cpuid4_cache_lookup(int index, struct cpuid4_info *this_leaf)
{
union _cpuid4_leaf_eax eax;
@@ -123,7 +121,7 @@ unsigned int init_intel_cacheinfo(struct
unsigned int trace = 0, l1i = 0, l1d = 0, l2 = 0, l3 = 0; /* Cache
sizes */
unsigned int new_l1d = 0, new_l1i = 0; /* Cache sizes from cpuid(4) */
unsigned int new_l2 = 0, new_l3 = 0, i; /* Cache sizes from cpuid(4) */
- unsigned int l2_id = 0, l3_id = 0, num_threads_sharing, index_msb;
+ static unsigned int num_cache_leaves;
if (c->cpuid_level > 3) {
static int is_initialized;
@@ -156,15 +154,9 @@ unsigned int init_intel_cacheinfo(struct
break;
case 2:
new_l2 = this_leaf.size/1024;
- num_threads_sharing = 1 +
this_leaf.eax.split.num_threads_sharing;
- index_msb =
get_count_order(num_threads_sharing);
- l2_id = c->apicid >> index_msb;
break;
case 3:
new_l3 = this_leaf.size/1024;
- num_threads_sharing = 1 +
this_leaf.eax.split.num_threads_sharing;
- index_msb =
get_count_order(num_threads_sharing);
- l3_id = c->apicid >> index_msb;
break;
default:
break;
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
