Re: [Xen-devel] [PATCH v6 1/5] xen/arm: observe itargets setting in vgic_enable_irqs and vgic_disable_irqs

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-07-02 at 16:39 +0100, Stefano Stabellini wrote:
> On Fri, 27 Jun 2014, Ian Campbell wrote:
> > On Mon, 2014-06-23 at 17:37 +0100, Stefano Stabellini wrote:
> > > @@ -589,12 +628,27 @@ static int vgic_distr_mmio_write(struct vcpu *v, 
> > > mmio_info_t *info)
> > >          if ( dabt.size != 0 && dabt.size != 2 ) goto bad_width;
> > >          rank = vgic_rank_offset(v, 8, gicd_reg - GICD_ITARGETSR);
> > >          if ( rank == NULL) goto write_ignore;
> > > +        /* 8-bit vcpu mask for this domain */
> > > +        BUG_ON(v->domain->max_vcpus > 8);
> > > +        tr = (1 << v->domain->max_vcpus) - 1;
> > > +        if ( dabt.size == 2 )
> > > +            tr = tr | (tr << 8) | (tr << 16) | (tr << 24);
> > > +        else
> > > +            tr = (tr << (8 * (offset & 0x3)));
> > > +        tr &= *r;
> > > +        /* ignore zero writes */
> > > +        if ( !tr )
> > > +            goto write_ignore;
> > 
> > Please can you add a comment here like:
> > 
> >                 /* For word reads ignore writes where any single byte is 
> > zero */
> > 
> > With that: Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > 
> > Although, might it be more reasonable to use the existing value for such
> > bytes? (i.e. only ignore the zero bytes, not the whole thing)
> 
> I don't know.. I would consider the entire write as invalid as it
> containts some invalid configurations.

The question is what does the spec say?

Ian.



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