1: make cpu_2_logical_apicid private to x2apic code 2: x2apic: properly implement cluster mode Signed-off-by: Jan Beulich <jbeulich@suse.com>
Jan Beulich
2012-Nov-08 15:02 UTC
[PATCH 1/2] x86: make cpu_2_logical_apicid private to x2apic code
... as it in fact is only being used there. While moving it, also make
it a per-CPU variable rather than a NR_CPUS-sized array.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -1024,7 +1024,6 @@ __next:
if (boot_cpu_physical_apicid == -1U)
boot_cpu_physical_apicid = get_apic_id();
x86_cpu_to_apicid[0] = get_apic_id();
- cpu_2_logical_apicid[0] = get_logical_apic_id();
init_ioapic_mappings();
}
--- a/xen/arch/x86/genapic/x2apic.c
+++ b/xen/arch/x86/genapic/x2apic.c
@@ -32,14 +32,15 @@
static bool_t __initdata x2apic_phys; /* By default we use logical cluster
mode. */
boolean_param("x2apic_phys", x2apic_phys);
+static DEFINE_PER_CPU_READ_MOSTLY(u32, cpu_2_logical_apicid);
+
static void init_apic_ldr_x2apic_phys(void)
{
}
static void init_apic_ldr_x2apic_cluster(void)
{
- int cpu = smp_processor_id();
- cpu_2_logical_apicid[cpu] = apic_read(APIC_LDR);
+ this_cpu(cpu_2_logical_apicid) = apic_read(APIC_LDR);
}
static void __init clustered_apic_check_x2apic(void)
@@ -48,7 +49,7 @@ static void __init clustered_apic_check_
static unsigned int cpu_mask_to_apicid_x2apic_cluster(const cpumask_t *cpumask)
{
- return cpu_2_logical_apicid[cpumask_first(cpumask)];
+ return per_cpu(cpu_2_logical_apicid, cpumask_first(cpumask));
}
static void __send_IPI_mask_x2apic(
@@ -77,7 +78,7 @@ static void __send_IPI_mask_x2apic(
if ( !cpu_online(cpu) || (cpu == smp_processor_id()) )
continue;
msr_content = (dest_mode == APIC_DEST_PHYSICAL)
- ? cpu_physical_id(cpu) : cpu_2_logical_apicid[cpu];
+ ? cpu_physical_id(cpu) : per_cpu(cpu_2_logical_apicid, cpu);
msr_content = (msr_content << 32) | APIC_DM_FIXED | dest_mode |
vector;
apic_wrmsr(APIC_ICR, msr_content);
}
--- a/xen/arch/x86/smp.c
+++ b/xen/arch/x86/smp.c
@@ -28,11 +28,6 @@ int hard_smp_processor_id(void)
return get_apic_id();
}
-int logical_smp_processor_id(void)
-{
- return get_logical_apic_id();
-}
-
/*
* send_IPI_mask(cpumask, vector): sends @vector IPI to CPUs in @cpumask,
* excluding the local CPU. @cpumask may be empty.
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -64,8 +64,6 @@ struct cpuinfo_x86 cpu_data[NR_CPUS];
u32 x86_cpu_to_apicid[NR_CPUS] __read_mostly { [0 ... NR_CPUS-1] = BAD_APICID
};
-static void map_cpu_to_logical_apicid(void);
-
static int cpu_error;
static enum cpu_state {
CPU_STATE_DYING, /* slave -> master: I am dying */
@@ -202,7 +200,6 @@ static void smp_callin(void)
Dprintk("CALLIN, before setup_local_APIC().\n");
x2apic_ap_setup();
setup_local_APIC();
- map_cpu_to_logical_apicid();
/* Save our processor parameters. */
smp_store_cpu_info(cpu);
@@ -401,22 +398,6 @@ extern struct {
unsigned short ss;
} stack_start;
-u32 cpu_2_logical_apicid[NR_CPUS] __read_mostly - { [0 ... NR_CPUS-1] =
BAD_APICID };
-
-static void map_cpu_to_logical_apicid(void)
-{
- int cpu = smp_processor_id();
- int apicid = logical_smp_processor_id();
-
- cpu_2_logical_apicid[cpu] = apicid;
-}
-
-static void unmap_cpu_to_logical_apicid(int cpu)
-{
- cpu_2_logical_apicid[cpu] = BAD_APICID;
-}
-
static int wakeup_secondary_cpu(int phys_apicid, unsigned long start_eip)
{
unsigned long send_status = 0, accept_status = 0;
@@ -646,7 +627,6 @@ static int do_boot_cpu(int apicid, int c
void cpu_exit_clear(unsigned int cpu)
{
cpu_uninit(cpu);
- unmap_cpu_to_logical_apicid(cpu);
set_cpu_state(CPU_STATE_DEAD);
}
@@ -775,7 +755,6 @@ void __init smp_prepare_cpus(unsigned in
if (APIC_init_uniprocessor())
printk(KERN_NOTICE "Local APIC not detected."
" Using dummy APIC emulation.\n");
- map_cpu_to_logical_apicid();
return;
}
@@ -804,7 +783,6 @@ void __init smp_prepare_cpus(unsigned in
connect_bsp_APIC();
setup_local_APIC();
- map_cpu_to_logical_apicid();
smpboot_setup_io_apic();
--- a/xen/include/asm-x86/apic.h
+++ b/xen/include/asm-x86/apic.h
@@ -158,12 +158,6 @@ static __inline u32 get_apic_id(void) /*
return x2apic_enabled ? id : GET_xAPIC_ID(id);
}
-static __inline u32 get_logical_apic_id(void)
-{
- u32 logical_id = apic_read(APIC_LDR);
- return x2apic_enabled ? logical_id : GET_xAPIC_LOGICAL_ID(logical_id);
-}
-
void apic_wait_icr_idle(void);
int get_physical_broadcast(void);
--- a/xen/include/asm-x86/smp.h
+++ b/xen/include/asm-x86/smp.h
@@ -37,7 +37,6 @@ extern void zap_low_mappings(void);
#define MAX_APICID 256
extern u32 x86_cpu_to_apicid[];
-extern u32 cpu_2_logical_apicid[];
#define cpu_physical_id(cpu) x86_cpu_to_apicid[cpu]
@@ -54,7 +53,6 @@ int cpu_add(uint32_t apic_id, uint32_t a
#define raw_smp_processor_id() (get_processor_id())
int hard_smp_processor_id(void);
-int logical_smp_processor_id(void);
void __stop_this_cpu(void);
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
So far, cluster mode was just an alternative implementation of
physical mode: Allowing only single CPU interrupt targets, and sending
IPIs to each target CPU separately. Take advantage of what cluster
mode really can do in that regard.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/genapic/x2apic.c
+++ b/xen/arch/x86/genapic/x2apic.c
@@ -19,6 +19,7 @@
#include <xen/config.h>
#include <xen/init.h>
+#include <xen/cpu.h>
#include <xen/cpumask.h>
#include <asm/apicdef.h>
#include <asm/genapic.h>
@@ -33,6 +34,14 @@ static bool_t __initdata x2apic_phys; /*
boolean_param("x2apic_phys", x2apic_phys);
static DEFINE_PER_CPU_READ_MOSTLY(u32, cpu_2_logical_apicid);
+static DEFINE_PER_CPU_READ_MOSTLY(cpumask_t *, cluster_cpus);
+static cpumask_t *cluster_cpus_spare;
+static DEFINE_PER_CPU(cpumask_var_t, scratch_mask);
+
+static inline u32 x2apic_cluster(unsigned int cpu)
+{
+ return per_cpu(cpu_2_logical_apicid, cpu) >> 16;
+}
static void init_apic_ldr_x2apic_phys(void)
{
@@ -40,20 +49,53 @@ static void init_apic_ldr_x2apic_phys(vo
static void init_apic_ldr_x2apic_cluster(void)
{
- this_cpu(cpu_2_logical_apicid) = apic_read(APIC_LDR);
+ unsigned int cpu, this_cpu = smp_processor_id();
+
+ per_cpu(cpu_2_logical_apicid, this_cpu) = apic_read(APIC_LDR);
+
+ if ( per_cpu(cluster_cpus, this_cpu) )
+ {
+ ASSERT(cpumask_test_cpu(this_cpu, per_cpu(cluster_cpus, this_cpu)));
+ return;
+ }
+
+ per_cpu(cluster_cpus, this_cpu) = cluster_cpus_spare;
+ for_each_online_cpu ( cpu )
+ {
+ if (this_cpu == cpu || x2apic_cluster(this_cpu) != x2apic_cluster(cpu))
+ continue;
+ per_cpu(cluster_cpus, this_cpu) = per_cpu(cluster_cpus, cpu);
+ break;
+ }
+ if ( per_cpu(cluster_cpus, this_cpu) == cluster_cpus_spare )
+ cluster_cpus_spare = NULL;
+
+ cpumask_set_cpu(this_cpu, per_cpu(cluster_cpus, this_cpu));
}
static void __init clustered_apic_check_x2apic(void)
{
}
+static const cpumask_t *vector_allocation_cpumask_x2apic_cluster(int cpu)
+{
+ return per_cpu(cluster_cpus, cpu);
+}
+
static unsigned int cpu_mask_to_apicid_x2apic_cluster(const cpumask_t *cpumask)
{
- return per_cpu(cpu_2_logical_apicid, cpumask_first(cpumask));
+ unsigned int cpu = cpumask_first(cpumask);
+ unsigned int dest = per_cpu(cpu_2_logical_apicid, cpu);
+ const cpumask_t *cluster_cpus = per_cpu(cluster_cpus, cpu);
+
+ for_each_cpu ( cpu, cluster_cpus )
+ if ( cpumask_test_cpu(cpu, cpumask) )
+ dest |= per_cpu(cpu_2_logical_apicid, cpu);
+
+ return dest;
}
-static void __send_IPI_mask_x2apic(
- const cpumask_t *cpumask, int vector, unsigned int dest_mode)
+static void send_IPI_mask_x2apic_phys(const cpumask_t *cpumask, int vector)
{
unsigned int cpu;
unsigned long flags;
@@ -77,23 +119,48 @@ static void __send_IPI_mask_x2apic(
{
if ( !cpu_online(cpu) || (cpu == smp_processor_id()) )
continue;
- msr_content = (dest_mode == APIC_DEST_PHYSICAL)
- ? cpu_physical_id(cpu) : per_cpu(cpu_2_logical_apicid, cpu);
- msr_content = (msr_content << 32) | APIC_DM_FIXED | dest_mode |
vector;
+ msr_content = cpu_physical_id(cpu);
+ msr_content = (msr_content << 32) | APIC_DM_FIXED |
+ APIC_DEST_PHYSICAL | vector;
apic_wrmsr(APIC_ICR, msr_content);
}
local_irq_restore(flags);
}
-static void send_IPI_mask_x2apic_phys(const cpumask_t *cpumask, int vector)
-{
- __send_IPI_mask_x2apic(cpumask, vector, APIC_DEST_PHYSICAL);
-}
-
static void send_IPI_mask_x2apic_cluster(const cpumask_t *cpumask, int vector)
{
- __send_IPI_mask_x2apic(cpumask, vector, APIC_DEST_LOGICAL);
+ unsigned int cpu = smp_processor_id();
+ cpumask_t *ipimask = per_cpu(scratch_mask, cpu);
+ const cpumask_t *cluster_cpus;
+ unsigned long flags;
+
+ mb(); /* See above for an explanation. */
+
+ local_irq_save(flags);
+
+ cpumask_andnot(ipimask, &cpu_online_map, cpumask_of(cpu));
+
+ for ( cpumask_and(ipimask, cpumask, ipimask); !cpumask_empty(ipimask);
+ cpumask_andnot(ipimask, ipimask, cluster_cpus) )
+ {
+ uint64_t msr_content = 0;
+
+ cluster_cpus = per_cpu(cluster_cpus, cpumask_first(ipimask));
+ for_each_cpu ( cpu, cluster_cpus )
+ {
+ if ( !cpumask_test_cpu(cpu, ipimask) )
+ continue;
+ msr_content |= per_cpu(cpu_2_logical_apicid, cpu);
+ }
+
+ BUG_ON(!msr_content);
+ msr_content = (msr_content << 32) | APIC_DM_FIXED |
+ APIC_DEST_LOGICAL | vector;
+ apic_wrmsr(APIC_ICR, msr_content);
+ }
+
+ local_irq_restore(flags);
}
static const struct genapic apic_x2apic_phys = {
@@ -116,15 +183,60 @@ static const struct genapic apic_x2apic_
.init_apic_ldr = init_apic_ldr_x2apic_cluster,
.clustered_apic_check = clustered_apic_check_x2apic,
.target_cpus = target_cpus_all,
- .vector_allocation_cpumask = vector_allocation_cpumask_phys,
+ .vector_allocation_cpumask = vector_allocation_cpumask_x2apic_cluster,
.cpu_mask_to_apicid = cpu_mask_to_apicid_x2apic_cluster,
.send_IPI_mask = send_IPI_mask_x2apic_cluster,
.send_IPI_self = send_IPI_self_x2apic
};
+static int update_clusterinfo(
+ struct notifier_block *nfb, unsigned long action, void *hcpu)
+{
+ unsigned int cpu = (unsigned long)hcpu;
+ int err = 0;
+
+ switch (action) {
+ case CPU_UP_PREPARE:
+ per_cpu(cpu_2_logical_apicid, cpu) = BAD_APICID;
+ if ( !cluster_cpus_spare )
+ cluster_cpus_spare = xzalloc(cpumask_t);
+ if ( !cluster_cpus_spare ||
+ !alloc_cpumask_var(&per_cpu(scratch_mask, cpu)) )
+ err = -ENOMEM;
+ break;
+ case CPU_UP_CANCELED:
+ case CPU_DEAD:
+ if ( per_cpu(cluster_cpus, cpu) )
+ {
+ cpumask_clear_cpu(cpu, per_cpu(cluster_cpus, cpu));
+ if ( cpumask_empty(per_cpu(cluster_cpus, cpu)) )
+ xfree(per_cpu(cluster_cpus, cpu));
+ }
+ free_cpumask_var(per_cpu(scratch_mask, cpu));
+ break;
+ }
+
+ return !err ? NOTIFY_DONE : notifier_from_errno(err);
+}
+
+static struct notifier_block x2apic_cpu_nfb = {
+ .notifier_call = update_clusterinfo
+};
+
const struct genapic *__init apic_x2apic_probe(void)
{
- return x2apic_phys ? &apic_x2apic_phys : &apic_x2apic_cluster;
+ if ( x2apic_phys )
+ return &apic_x2apic_phys;
+
+ if ( !this_cpu(cluster_cpus) )
+ {
+ update_clusterinfo(NULL, CPU_UP_PREPARE,
+ (void *)(long)smp_processor_id());
+ init_apic_ldr_x2apic_cluster();
+ register_cpu_notifier(&x2apic_cpu_nfb);
+ }
+
+ return &apic_x2apic_cluster;
}
void __init check_x2apic_preenabled(void)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Keir Fraser
2012-Nov-08 15:17 UTC
Re: [PATCH 2/2] x86/x2apic: properly implement cluster mode
On 08/11/2012 15:03, "Jan Beulich" <JBeulich@suse.com> wrote:> So far, cluster mode was just an alternative implementation of > physical mode: Allowing only single CPU interrupt targets, and sending > IPIs to each target CPU separately. Take advantage of what cluster > mode really can do in that regard.What does it allow? Multicast within certain constraints? I know it''s not part of our coding style, but some comments would be nice. ;) -- Keir> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/arch/x86/genapic/x2apic.c > +++ b/xen/arch/x86/genapic/x2apic.c > @@ -19,6 +19,7 @@ > > #include <xen/config.h> > #include <xen/init.h> > +#include <xen/cpu.h> > #include <xen/cpumask.h> > #include <asm/apicdef.h> > #include <asm/genapic.h> > @@ -33,6 +34,14 @@ static bool_t __initdata x2apic_phys; /* > boolean_param("x2apic_phys", x2apic_phys); > > static DEFINE_PER_CPU_READ_MOSTLY(u32, cpu_2_logical_apicid); > +static DEFINE_PER_CPU_READ_MOSTLY(cpumask_t *, cluster_cpus); > +static cpumask_t *cluster_cpus_spare; > +static DEFINE_PER_CPU(cpumask_var_t, scratch_mask); > + > +static inline u32 x2apic_cluster(unsigned int cpu) > +{ > + return per_cpu(cpu_2_logical_apicid, cpu) >> 16; > +} > > static void init_apic_ldr_x2apic_phys(void) > { > @@ -40,20 +49,53 @@ static void init_apic_ldr_x2apic_phys(vo > > static void init_apic_ldr_x2apic_cluster(void) > { > - this_cpu(cpu_2_logical_apicid) = apic_read(APIC_LDR); > + unsigned int cpu, this_cpu = smp_processor_id(); > + > + per_cpu(cpu_2_logical_apicid, this_cpu) = apic_read(APIC_LDR); > + > + if ( per_cpu(cluster_cpus, this_cpu) ) > + { > + ASSERT(cpumask_test_cpu(this_cpu, per_cpu(cluster_cpus, this_cpu))); > + return; > + } > + > + per_cpu(cluster_cpus, this_cpu) = cluster_cpus_spare; > + for_each_online_cpu ( cpu ) > + { > + if (this_cpu == cpu || x2apic_cluster(this_cpu) !> x2apic_cluster(cpu)) > + continue; > + per_cpu(cluster_cpus, this_cpu) = per_cpu(cluster_cpus, cpu); > + break; > + } > + if ( per_cpu(cluster_cpus, this_cpu) == cluster_cpus_spare ) > + cluster_cpus_spare = NULL; > + > + cpumask_set_cpu(this_cpu, per_cpu(cluster_cpus, this_cpu)); > } > > static void __init clustered_apic_check_x2apic(void) > { > } > > +static const cpumask_t *vector_allocation_cpumask_x2apic_cluster(int cpu) > +{ > + return per_cpu(cluster_cpus, cpu); > +} > + > static unsigned int cpu_mask_to_apicid_x2apic_cluster(const cpumask_t > *cpumask) > { > - return per_cpu(cpu_2_logical_apicid, cpumask_first(cpumask)); > + unsigned int cpu = cpumask_first(cpumask); > + unsigned int dest = per_cpu(cpu_2_logical_apicid, cpu); > + const cpumask_t *cluster_cpus = per_cpu(cluster_cpus, cpu); > + > + for_each_cpu ( cpu, cluster_cpus ) > + if ( cpumask_test_cpu(cpu, cpumask) ) > + dest |= per_cpu(cpu_2_logical_apicid, cpu); > + > + return dest; > } > > -static void __send_IPI_mask_x2apic( > - const cpumask_t *cpumask, int vector, unsigned int dest_mode) > +static void send_IPI_mask_x2apic_phys(const cpumask_t *cpumask, int vector) > { > unsigned int cpu; > unsigned long flags; > @@ -77,23 +119,48 @@ static void __send_IPI_mask_x2apic( > { > if ( !cpu_online(cpu) || (cpu == smp_processor_id()) ) > continue; > - msr_content = (dest_mode == APIC_DEST_PHYSICAL) > - ? cpu_physical_id(cpu) : per_cpu(cpu_2_logical_apicid, cpu); > - msr_content = (msr_content << 32) | APIC_DM_FIXED | dest_mode | > vector; > + msr_content = cpu_physical_id(cpu); > + msr_content = (msr_content << 32) | APIC_DM_FIXED | > + APIC_DEST_PHYSICAL | vector; > apic_wrmsr(APIC_ICR, msr_content); > } > > local_irq_restore(flags); > } > > -static void send_IPI_mask_x2apic_phys(const cpumask_t *cpumask, int vector) > -{ > - __send_IPI_mask_x2apic(cpumask, vector, APIC_DEST_PHYSICAL); > -} > - > static void send_IPI_mask_x2apic_cluster(const cpumask_t *cpumask, int > vector) > { > - __send_IPI_mask_x2apic(cpumask, vector, APIC_DEST_LOGICAL); > + unsigned int cpu = smp_processor_id(); > + cpumask_t *ipimask = per_cpu(scratch_mask, cpu); > + const cpumask_t *cluster_cpus; > + unsigned long flags; > + > + mb(); /* See above for an explanation. */ > + > + local_irq_save(flags); > + > + cpumask_andnot(ipimask, &cpu_online_map, cpumask_of(cpu)); > + > + for ( cpumask_and(ipimask, cpumask, ipimask); !cpumask_empty(ipimask); > + cpumask_andnot(ipimask, ipimask, cluster_cpus) ) > + { > + uint64_t msr_content = 0; > + > + cluster_cpus = per_cpu(cluster_cpus, cpumask_first(ipimask)); > + for_each_cpu ( cpu, cluster_cpus ) > + { > + if ( !cpumask_test_cpu(cpu, ipimask) ) > + continue; > + msr_content |= per_cpu(cpu_2_logical_apicid, cpu); > + } > + > + BUG_ON(!msr_content); > + msr_content = (msr_content << 32) | APIC_DM_FIXED | > + APIC_DEST_LOGICAL | vector; > + apic_wrmsr(APIC_ICR, msr_content); > + } > + > + local_irq_restore(flags); > } > > static const struct genapic apic_x2apic_phys = { > @@ -116,15 +183,60 @@ static const struct genapic apic_x2apic_ > .init_apic_ldr = init_apic_ldr_x2apic_cluster, > .clustered_apic_check = clustered_apic_check_x2apic, > .target_cpus = target_cpus_all, > - .vector_allocation_cpumask = vector_allocation_cpumask_phys, > + .vector_allocation_cpumask = vector_allocation_cpumask_x2apic_cluster, > .cpu_mask_to_apicid = cpu_mask_to_apicid_x2apic_cluster, > .send_IPI_mask = send_IPI_mask_x2apic_cluster, > .send_IPI_self = send_IPI_self_x2apic > }; > > +static int update_clusterinfo( > + struct notifier_block *nfb, unsigned long action, void *hcpu) > +{ > + unsigned int cpu = (unsigned long)hcpu; > + int err = 0; > + > + switch (action) { > + case CPU_UP_PREPARE: > + per_cpu(cpu_2_logical_apicid, cpu) = BAD_APICID; > + if ( !cluster_cpus_spare ) > + cluster_cpus_spare = xzalloc(cpumask_t); > + if ( !cluster_cpus_spare || > + !alloc_cpumask_var(&per_cpu(scratch_mask, cpu)) ) > + err = -ENOMEM; > + break; > + case CPU_UP_CANCELED: > + case CPU_DEAD: > + if ( per_cpu(cluster_cpus, cpu) ) > + { > + cpumask_clear_cpu(cpu, per_cpu(cluster_cpus, cpu)); > + if ( cpumask_empty(per_cpu(cluster_cpus, cpu)) ) > + xfree(per_cpu(cluster_cpus, cpu)); > + } > + free_cpumask_var(per_cpu(scratch_mask, cpu)); > + break; > + } > + > + return !err ? NOTIFY_DONE : notifier_from_errno(err); > +} > + > +static struct notifier_block x2apic_cpu_nfb = { > + .notifier_call = update_clusterinfo > +}; > + > const struct genapic *__init apic_x2apic_probe(void) > { > - return x2apic_phys ? &apic_x2apic_phys : &apic_x2apic_cluster; > + if ( x2apic_phys ) > + return &apic_x2apic_phys; > + > + if ( !this_cpu(cluster_cpus) ) > + { > + update_clusterinfo(NULL, CPU_UP_PREPARE, > + (void *)(long)smp_processor_id()); > + init_apic_ldr_x2apic_cluster(); > + register_cpu_notifier(&x2apic_cpu_nfb); > + } > + > + return &apic_x2apic_cluster; > } > > void __init check_x2apic_preenabled(void) > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
On 08/11/2012 14:59, "Jan Beulich" <JBeulich@suse.com> wrote:> 1: make cpu_2_logical_apicid private to x2apic code > 2: x2apic: properly implement cluster mode > > Signed-off-by: Jan Beulich <jbeulich@suse.com>Had a minor comment on patch 2, but apart from that... Acked-by: Keir Fraser <keir@xen.org>> > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Jan Beulich
2012-Nov-08 15:38 UTC
Re: [PATCH 2/2] x86/x2apic: properly implement cluster mode
>>> On 08.11.12 at 16:17, Keir Fraser <keir@xen.org> wrote: > On 08/11/2012 15:03, "Jan Beulich" <JBeulich@suse.com> wrote: > >> So far, cluster mode was just an alternative implementation of >> physical mode: Allowing only single CPU interrupt targets, and sending >> IPIs to each target CPU separately. Take advantage of what cluster >> mode really can do in that regard. > > What does it allow? Multicast within certain constraints?For IPIs, yes. For interrupts, this gets things merely in line with cluster mode APIC, specifying more than one destination with lowest priority delivery. Without actively using the TPR, that should only affect nested interrupts (which previously would have got deferred until the outer one finished if their vector was a lower priority one than the one in service, whereas now they could get serviced by another CPU within the cluster). I also should have pointed out that this has a potential drawback - it might increase the pressure on vector allocation, since they''re now being shared more heavily (within clusters). But if someone runs into problems with that, they ought to simply switch to physical mode (which previously really had no practical use at all).> I know it''s not > part of our coding style, but some comments would be nice. ;)Hard to tell what specifically you would want to see commented, given that this really just extends the originally rather simplistic implementation. Jan
Keir Fraser
2012-Nov-08 16:52 UTC
Re: [PATCH 2/2] x86/x2apic: properly implement cluster mode
On 08/11/2012 15:38, "Jan Beulich" <JBeulich@suse.com> wrote:>>>> On 08.11.12 at 16:17, Keir Fraser <keir@xen.org> wrote: >> On 08/11/2012 15:03, "Jan Beulich" <JBeulich@suse.com> wrote: >> >>> So far, cluster mode was just an alternative implementation of >>> physical mode: Allowing only single CPU interrupt targets, and sending >>> IPIs to each target CPU separately. Take advantage of what cluster >>> mode really can do in that regard. >> >> What does it allow? Multicast within certain constraints? > > For IPIs, yes. For interrupts, this gets things merely in line with > cluster mode APIC, specifying more than one destination with > lowest priority delivery. Without actively using the TPR, that > should only affect nested interrupts (which previously would > have got deferred until the outer one finished if their vector > was a lower priority one than the one in service, whereas now > they could get serviced by another CPU within the cluster). > > I also should have pointed out that this has a potential drawback > - it might increase the pressure on vector allocation, since > they''re now being shared more heavily (within clusters). But if > someone runs into problems with that, they ought to simply > switch to physical mode (which previously really had no practical > use at all). > >> I know it''s not >> part of our coding style, but some comments would be nice. ;) > > Hard to tell what specifically you would want to see commented, > given that this really just extends the originally rather simplistic > implementation.Okay, yes, it doesn''t look so complicated re-reading it again. I''m fine with it as it is. -- Keir> Jan >