[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v8 4/8] xen/arm: Use AFF1 when translating ICC_SGI1R_EL1 to cpumask
On Wed, 2015-06-17 at 14:13 +0100, Julien Grall wrote: > On 17/06/15 14:00, Ian Campbell wrote: > > On Fri, 2015-06-12 at 16:32 +0800, Chen Baozi wrote: > >> From: Chen Baozi <baozich@xxxxxxxxx> > >> > >> The old unsigned long type of vcpu_mask can only express 64 cpus at the > >> most, which might not be enough for the guest which used vGICv3. We > >> introduce a new struct sgi_target for the target cpu list of SGI, which > >> holds the affinity path information (only level 1 at the moment). For > >> GICv2 that has no affinity level, we can just set the corresponding > >> fields to be 0. > >> > >> Signed-off-by: Chen Baozi <baozich@xxxxxxxxx> > >> --- > >> xen/arch/arm/vgic-v2.c | 7 +++--- > >> xen/arch/arm/vgic-v3.c | 10 +++++---- > >> xen/arch/arm/vgic.c | 45 > >> +++++++++++++++++---------------------- > >> xen/include/asm-arm/gic_v3_defs.h | 3 +++ > >> xen/include/asm-arm/vgic.h | 7 +++++- > >> 5 files changed, 38 insertions(+), 34 deletions(-) > >> > >> diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c > >> index 3be1a51..5949cf1 100644 > >> --- a/xen/arch/arm/vgic-v2.c > >> +++ b/xen/arch/arm/vgic-v2.c > >> @@ -201,16 +201,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; > >> + struct sgi_target target; > >> > >> + memset(&target, 0, sizeof(struct sgi_target)); > > > > I'd prefer explicit initialisation of the relevant fields please. Which > > may mean setting aff1 to 0 somewhere at the top, with a suitable comment > > as to why, and might involve setting target.list to zero in some other > > cases below or via an explicit initialiser here. > > Well, only SGI_TARGET_LIST is caring about struct sgi_target (see > vgic_to_sgi). I would only initialize it when it's required. Good point, and by keeping it a pointer you could even pass NULL in the other cases, making this more obvious still. > > > >> diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > >> index 7b387b7..59bd98a 100644 > >> --- a/xen/arch/arm/vgic.c > >> +++ b/xen/arch/arm/vgic.c > >> @@ -318,15 +318,14 @@ 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) > >> + const struct sgi_target *target) > > > > For a 3 byte struct perhaps we can pass by value instead of reference? > > > > I suppose it might eventually be 5 bytes, but even so... > > > >> @@ -334,29 +333,33 @@ int vgic_to_sgi(struct vcpu *v, register_t sgir, > >> enum gic_sgi_mode irqmode, int > >> { > >> case SGI_TARGET_LIST: > >> perfc_incr(vgic_sgi_list); > >> + base = target->aff1 << 4; > >> + bitmap = target->list; > >> + for_each_set_bit( i, &bitmap, sizeof(target->list) * 8 ) > >> + { > >> + vcpuid = base + i; > >> + if ( d->vcpu[vcpuid] != NULL && > >> !is_vcpu_online(d->vcpu[vcpuid]) ) > > > > What if d->vcpu[vcpuid] is NULL? (Was this a latent bug before, or am I > > missing something?) > > I don't see any problem, if d->vcpu[vcpuid] is NULL there is no need to > send an SGI as the VCPU is not present. But the code will, I think. I should have quoted a bit more, briefly it is : + if ( d->vcpu[vcpuid] != NULL && !is_vcpu_online(d->vcpu[vcpuid]) ) continue + vgic_vcpu_inject_irq(d->vcpu[vcpuid], virq); So if d->vcpu[vcpuid] == NULL it will try and send an SGI to it, won't it? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |