Re: [Xen-devel] [PATCH 2/4] xen/arm: support for guest SGI

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-04-23 at 12:50 +0100, Stefano Stabellini wrote:
> On Tue, 9 Apr 2013, Ian Campbell wrote:
> > On Thu, 2013-03-21 at 18:42 +0000, Stefano Stabellini wrote:
> > > Trap writes to GICD_SGIR, parse the requests, inject SGIs into the right
> > > guest vcpu.
> > > 
> > > Add a useful debug printk to vgic_vcpu_inject_irq.
> > > 
> > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > > ---
> > >  xen/arch/arm/vgic.c       |   53 
> > > +++++++++++++++++++++++++++++++++++++++++---
> > >  xen/include/asm-arm/gic.h |    3 ++
> > >  2 files changed, 52 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> > > index 8495384..26279f9 100644
> > > --- a/xen/arch/arm/vgic.c
> > > +++ b/xen/arch/arm/vgic.c
> > > @@ -370,6 +370,7 @@ static void vgic_enable_irqs(struct vcpu *v, uint32_t 
> > > r, int n)
> > >  
> > >  static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info)
> > >  {
> > > +    struct domain *d = v->domain;
> > >      struct hsr_dabt dabt = info->dabt;
> > >      struct cpu_user_regs *regs = guest_cpu_user_regs();
> > >      uint32_t *r = select_user_reg(regs, dabt.reg);
> > > @@ -498,10 +499,51 @@ static int vgic_distr_mmio_write(struct vcpu *v, 
> > > mmio_info_t *info)
> > >          goto write_ignore;
> > >  
> > >      case GICD_SGIR:
> > > -        if ( dabt.size != 2 ) goto bad_width;
> > > -        printk("vGICD: unhandled write %#"PRIx32" to ICFGR%d\n",
> > > -               *r, gicd_reg - GICD_ICFGR);
> > > -        return 0;
> > > +        {
> > > +            cpumask_t vcpu_mask;
> > > +            int virtual_irq;
> > > +            int filter;
> > > +            int vcpuid;
> > > +            struct vcpu *vt;
> > > +            int i;
> > > +
> > > +            if ( dabt.size != 2 ) goto bad_width;
> > > +
> > > +            filter = (*r & GICD_SGI_TARGET_LIST_MASK);
> > > +            virtual_irq = (*r & GICD_SGI_INTID_MASK);
> > > +
> > > +            cpumask_clear(&vcpu_mask);
> > > +            switch ( filter )
> > > +            {
> > > +                case GICD_SGI_TARGET_LIST:
> > > +                    cpumask_bits(&vcpu_mask)[0] = (*r & 
> > > GICD_SGI_TARGET_MASK) >> GICD_SGI_TARGET_SHIFT;
> > 
> > Is this the best available way to set a cpumask in Xen?
> 
> We could probably use cpumask_or to set the bits in vcpu_mask, but I
> don't think it would be a great improvement.

It would have the advantage of not breaking the cpumask abstraction
quite so wide open though.

> > > +                    break;
> > > +                case GICD_SGI_TARGET_OTHERS:
> > > +                    for ( i = 0; i < d->max_vcpus; i++ )
> > > +                    {
> > > +                        if ( i != current->vcpu_id && d->vcpu[i] != NULL 
> > > )
> > > +                            cpumask_set_cpu(i, &vcpu_mask);
> > 
> > Can this be done with cpumask_any/all and then clearing the current cpu?
> 
> cpumask_all doesn't actually exist and I don't see a way to use
> cpumask_any to improve this code.
> If we were dealing with pcpus, we could use cpu_online_map and
> cpumask_clear_cpu, but given that they are vcpus, I don't think we can
> do much better than this.

OK.



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