Re: [Xen-devel] [PATCH v4 04/12] xen/arm: support for guest SGI

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 30 Apr 2013, Ian Campbell wrote:
> On Tue, 2013-04-30 at 16:32 +0100, Stefano Stabellini wrote:
> > On Tue, 30 Apr 2013, Ian Campbell wrote:
> > > On Mon, 2013-04-29 at 18:51 +0100, Stefano Stabellini wrote:
> > > > On Mon, 29 Apr 2013, Stefano Stabellini wrote:
> > > > > On Mon, 29 Apr 2013, Ian Campbell wrote:
> > > > > > On Mon, 2013-04-29 at 16:47 +0100, Stefano Stabellini wrote:
> > > > > > > On Mon, 29 Apr 2013, Ian Campbell wrote:
> > > > > > > > On Sun, 2013-04-28 at 15:26 +0100, Stefano Stabellini wrote:
> > > > > > > > > On Fri, 26 Apr 2013, Ian Campbell wrote:
> > > > > > > > > > > +
> > > > > > > > > > > +
> > > > > > > > > > > +    for_each_set_bit( vcpuid, &vcpu_mask, d->max_vcpus )
> > > > > > > > > > > +    {
> > > > > > > > > > > +        if ( vcpuid >= d->max_vcpus || (vt = 
> > > > > > > > > > > d->vcpu[vcpuid]) == NULL )
> > > > > > > > > > 
> > > > > > > > > > Is d->vcpu[vcpuid] == NULL sufficient here? A vcpu which 
> > > > > > > > > > exists but has
> > > > > > > > > > not been PSCI'd up will pass this -- what do we do to them? 
> > > > > > > > > > Hopefully we
> > > > > > > > > > don't wake them up? Do we not want to check something like 
> > > > > > > > > > _VPF_down
> > > > > > > > > > too?
> > > > > > > > > 
> > > > > > > > > I think you are right, we should test for _VPF_down too
> > > > > > > > 
> > > > > > > > Are there any interesting races here? Seems like there should 
> > > > > > > > be, unless
> > > > > > > > we hold the domain lock or something?
> > > > > > > 
> > > > > > > 
> > > > > > > Considering that max_vcpus is static, d->vcpu[vcpuid] has been
> > > > > > > allocated at domain creation and that test_bit should be atomic, 
> > > > > > > I don't
> > > > > > > think there are any races here.
> > > > > > > What am I missing?
> > > > > > 
> > > > > > The race between checking the bit and acting on the result.
> > > > > 
> > > > > I don't think that race can cause any problems: if vcpu0 is sending a
> > > > > TARGET_OTHERS SGI and vcpu1 is executing psci.cpu_off, depending on 
> > > > > who
> > > > > wins the race vcpu1 is going to receive the SGI on not, but the result
> > > > > is going to be always consistent: if vcpu1 shuts down before the SGI 
> > > > > is
> > > > > sent, it is not going to receive it (actually it is going to receive 
> > > > > it
> > > > > if it gets ever woken up), otherwise it is going to receive it before
> > > > > shutting down.
> > > 
> > > Did you test the case where the SGI gets delivered to a CPU which was up
> > > but now is down? We don't want to crash because some resource has been
> > > freed etc...
> > 
> > I haven't actually tested it, but no resources are freed:
> > vcpu_sleep_nosync only sets a scheduler flag.
> 
> Good, so vcpus are only actually destroyed when we are destroying a
> domain.
> 
> Do we need to be checking d->is_dying?

I don't think so: is_dying is used for toolstack driven domain kill
operations.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel