Re: [Xen-devel] [PATCH 1/2] xen/arm: add a warning if the guest asks for SPI delivery to vcpu != 0.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Sun, 25 May 2014, Julien Grall wrote:
> Hi Stefano,
> 
> On 25/05/14 19:06, Stefano Stabellini wrote:
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > ---
> >   xen/arch/arm/vgic.c |    5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
> > index 4869b87..e4f38a0 100644
> > --- a/xen/arch/arm/vgic.c
> > +++ b/xen/arch/arm/vgic.c
> > @@ -581,6 +581,11 @@ 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_irq_rank(v, 8, gicd_reg - GICD_ITARGETSR);
> >           if ( rank == NULL) goto write_ignore;
> > +        if ( *r )
> 
> This is wrong, ITARGETSR store a  4 bitmask of CPUs, one per interrupt. Each
> bit of the mask correspond to a CPU (see Table 4-17 in the GICv2 manual).

you are right


> Furthermore, I think it's safe to just ignore write. The manual says:
> "It is IMPLEMENTATION DEFINED which, if any, SPIs are statically configured in
> hardware. The CPU targets field for such an SPI is read-only, and returns a
> value that indicates the CPU targets for the interrupt."

Good point


> With the former comment, your patch #2 is also wrong.

Patch #2 is valid regardless of the specific itarget setting


> > +        {
> > +            gdprintk(XENLOG_DEBUG, "SPI delivery to seconday cpus is
> > unimplemented\n");
> 
> s/seconday/secondary/
> 
> Regards,
> 
> -- 
> Julien Grall
> 

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