[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH V5 04/10] xen/arm: Use cpumask_t type for vcpu_mask in vgic_to_sgi
Hi Chen,
On 30/05/2015 12:07, Chen Baozi wrote:
From: Chen Baozi <baozich@xxxxxxxxx>
Use cpumask_t instead of unsigned long which can only express 64 cpus at
the most. Add the {gicv2|gicv3}_sgir_to_cpumask in corresponding vGICs
to translate GICD_SGIR/ICC_SGI1R_EL1 to vcpu_mask for vgic_to_sgi.
Signed-off-by: Chen Baozi <baozich@xxxxxxxxx>
---
xen/arch/arm/vgic-v2.c | 16 +++++++++++++---
xen/arch/arm/vgic-v3.c | 19 +++++++++++++++----
xen/arch/arm/vgic.c | 24 +++++++++++-------------
xen/include/asm-arm/gic.h | 1 +
xen/include/asm-arm/gic_v3_defs.h | 2 ++
xen/include/asm-arm/vgic.h | 2 +-
6 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 3be1a51..17a3c9f 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -33,6 +33,15 @@
#include <asm/gic.h>
#include <asm/vgic.h>
+static inline void gicv2_sgir_to_cpumask(cpumask_t *cpumask,
+ const register_t sgir)
+{
+ unsigned long target_list;
+
+ target_list = ((sgir & GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT);
+ bitmap_copy(cpumask_bits(cpumask), &target_list, GICD_SGI_TARGET_BITS);
+}
+
static int vgic_v2_distr_mmio_read(struct vcpu *v, mmio_info_t *info)
{
struct hsr_dabt dabt = info->dabt;
@@ -201,16 +210,17 @@ static int vgic_v2_to_sgi(struct vcpu *v, register_t sgir)
int virq;
int irqmode;
enum gic_sgi_mode sgi_mode;
- unsigned long vcpu_mask = 0;
+ cpumask_t vcpu_mask;
+ cpumask_clear(&vcpu_mask);
irqmode = (sgir & GICD_SGI_TARGET_LIST_MASK) >>
GICD_SGI_TARGET_LIST_SHIFT;
virq = (sgir & GICD_SGI_INTID_MASK);
- vcpu_mask = (sgir & GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT;
/* Map GIC sgi value to enum value */
switch ( irqmode )
{
case GICD_SGI_TARGET_LIST_VAL:
+ gicv2_sgir_to_cpumask(&vcpu_mask, sgir);
sgi_mode = SGI_TARGET_LIST;
break;
case GICD_SGI_TARGET_OTHERS_VAL:
@@ -226,7 +236,7 @@ static int vgic_v2_to_sgi(struct vcpu *v, register_t sgir)
return 0;
}
- return vgic_to_sgi(v, sgir, sgi_mode, virq, vcpu_mask);
+ return vgic_to_sgi(v, sgir, sgi_mode, virq, &vcpu_mask);
}
static int vgic_v2_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index ef9a71a..a283c8c 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -972,22 +972,33 @@ write_ignore:
return 1;
}
+static inline void gicv3_sgir_to_cpumask(cpumask_t *cpumask,
+ const register_t sgir)
+{
+ unsigned long target_list;
+
+ target_list = sgir & ICH_SGI_TARGETLIST_MASK;
+ bitmap_copy(cpumask_bits(cpumask), &target_list, ICH_SGI_TARGET_BITS);
+
Spurious line.
+}
+
static int vgic_v3_to_sgi(struct vcpu *v, register_t sgir)
{
int virq;
int irqmode;
enum gic_sgi_mode sgi_mode;
- unsigned long vcpu_mask = 0;
+ cpumask_t vcpu_mask;
+ cpumask_clear(&vcpu_mask);
irqmode = (sgir >> ICH_SGI_IRQMODE_SHIFT) & ICH_SGI_IRQMODE_MASK;
virq = (sgir >> ICH_SGI_IRQ_SHIFT ) & ICH_SGI_IRQ_MASK;
- /* SGI's are injected at Rdist level 0. ignoring affinity 1, 2, 3 */
- vcpu_mask = sgir & ICH_SGI_TARGETLIST_MASK;
/* Map GIC sgi value to enum value */
switch ( irqmode )
{
case ICH_SGI_TARGET_LIST:
+ /* SGI's are injected at Rdist level 0. ignoring affinity 1, 2, 3 */
+ gicv3_sgir_to_cpumask(&vcpu_mask, sgir);
sgi_mode = SGI_TARGET_LIST;
break;
case ICH_SGI_TARGET_OTHERS:
@@ -998,7 +1009,7 @@ static int vgic_v3_to_sgi(struct vcpu *v, register_t sgir)
return 0;
}
- return vgic_to_sgi(v, sgir, sgi_mode, virq, vcpu_mask);
+ return vgic_to_sgi(v, sgir, sgi_mode, virq, &vcpu_mask);
}
static int vgic_v3_emulate_sysreg(struct cpu_user_regs *regs, union hsr hsr)
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 7b387b7..f9e43f9 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -318,16 +318,12 @@ void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n)
}
}
-/* TODO: unsigned long is used to fit vcpu_mask.*/
int vgic_to_sgi(struct vcpu *v, register_t sgir, enum gic_sgi_mode irqmode,
int virq,
- unsigned long vcpu_mask)
+ cpumask_t *vcpu_mask)
{
struct domain *d = v->domain;
- int vcpuid;
int i;
- ASSERT(d->max_vcpus < 8*sizeof(vcpu_mask));
-
As said in the previous version, cpumask_t is based on NR_CPUS and there
is no relation between NR_CPUS and MAX_VIRT_CPUS. Furthermore, NR_CPUS
which can be configured at build time by the user (see MAX_PHYS_CPUS).
We need to catch at build time possible when the cpumask_t would be
smaller in order to avoid insecure hypervisor.
ASSERT( virq < 16 );
switch ( irqmode )
@@ -341,12 +337,12 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum
gic_sgi_mode irqmode, int
* SGI_TARGET_SELF mode. So Force vcpu_mask to 0
*/
perfc_incr(vgic_sgi_others);
- vcpu_mask = 0;
+ cpumask_clear(vcpu_mask);
Given that you already clear the cpumask on the caller of this function,
I think we can consider that it will be nice with us rather than wasting
CPU time to clear twice. That would require to update the comment above.
for ( i = 0; i < d->max_vcpus; i++ )
{
if ( i != current->vcpu_id && d->vcpu[i] != NULL &&
is_vcpu_online(d->vcpu[i]) )
- set_bit(i, &vcpu_mask);
+ cpumask_set_cpu(i, vcpu_mask);
}
break;
case SGI_TARGET_SELF:
@@ -355,8 +351,8 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum
gic_sgi_mode irqmode, int
* SGI_TARGET_SELF mode. So Force vcpu_mask to 0
*/
perfc_incr(vgic_sgi_self);
- vcpu_mask = 0;
- set_bit(current->vcpu_id, &vcpu_mask);
+ cpumask_clear(vcpu_mask);
Ditto.
+ cpumask_set_cpu(current->vcpu_id, vcpu_mask);
break;
default:
gprintk(XENLOG_WARNING,
@@ -365,15 +361,17 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, enum
gic_sgi_mode irqmode, int
return 0;
}
- for_each_set_bit( vcpuid, &vcpu_mask, d->max_vcpus )
+ for ( i = cpumask_first(vcpu_mask);
+ i < d->max_vcpus;
+ i = cpumask_next(i, vcpu_mask))
The indentation looks wrong here. And why don't you use vcpuid? It would
have avoid the renaming vcpuid/i below which increase the path size for
nothing.
{
- if ( d->vcpu[vcpuid] != NULL && !is_vcpu_online(d->vcpu[vcpuid]) )
+ if ( d->vcpu[i] != NULL && !is_vcpu_online(d->vcpu[i]) )
{
gprintk(XENLOG_WARNING, "VGIC: write r=%"PRIregister" \
- vcpu_mask=%lx, wrong CPUTargetList\n", sgir, vcpu_mask);
+ , wrong CPUTargetList\n", sgir);
continue;
}
- vgic_vcpu_inject_irq(d->vcpu[vcpuid], virq);
+ vgic_vcpu_inject_irq(d->vcpu[i], virq);
}
return 1;
}
diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h
index 463fffb..c6ef4bf 100644
--- a/xen/include/asm-arm/gic.h
+++ b/xen/include/asm-arm/gic.h
@@ -64,6 +64,7 @@
#define GICD_SGI_TARGET_SELF_VAL (2)
#define GICD_SGI_TARGET_SHIFT (16)
#define GICD_SGI_TARGET_MASK (0xFFUL<<GICD_SGI_TARGET_SHIFT)
+#define GICD_SGI_TARGET_BITS (8)
#define GICD_SGI_GROUP1 (1UL<<15)
#define GICD_SGI_INTID_MASK (0xFUL)
diff --git a/xen/include/asm-arm/gic_v3_defs.h
b/xen/include/asm-arm/gic_v3_defs.h
index 556f114..e106e67 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/gic_v3_defs.h
@@ -152,6 +152,8 @@
#define ICH_SGI_IRQ_SHIFT 24
#define ICH_SGI_IRQ_MASK 0xf
#define ICH_SGI_TARGETLIST_MASK 0xffff
+#define ICH_SGI_TARGET_BITS 16
+
#endif /* __ASM_ARM_GIC_V3_DEFS_H__ */
/*
diff --git a/xen/include/asm-arm/vgic.h b/xen/include/asm-arm/vgic.h
index 6dcdf9f..2f413e1 100644
--- a/xen/include/asm-arm/vgic.h
+++ b/xen/include/asm-arm/vgic.h
@@ -201,7 +201,7 @@ DEFINE_VGIC_OPS(3)
extern int vcpu_vgic_free(struct vcpu *v);
extern int vgic_to_sgi(struct vcpu *v, register_t sgir,
enum gic_sgi_mode irqmode, int virq,
- unsigned long vcpu_mask);
+ cpumask_t *vcpu_mask);
extern void vgic_migrate_irq(struct vcpu *old, struct vcpu *new, unsigned int
irq);
/* Reserve a specific guest vIRQ */
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
