| Message ID | 1401811627-4389-1-git-send-email-stefano.stabellini@eu.citrix.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 2014-06-03 at 17:07 +0100, Stefano Stabellini wrote: > Ignore guest writes to GICD_ITARGETSR that set the target cpu to a cpu > other than cpu0 for SPIs. > > Also ignore guest writes to GICD_ITARGETSR for PPIs and SGIs as they can > only be delivered to the same cpu and that has already been configured > at initialization time. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > Changes in v2: > - ignore writes to rank 0; > - don't print a warning for ignoring writes to GICD_ITARGETSR; > - add a comment in the code to remember that we don't implement writes > to GICD_ITARGETSR. > --- > xen/arch/arm/vgic.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index cb8df3a..1304b5e 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c > @@ -584,6 +584,13 @@ 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; > + /* only same vcpu delivery can be allowed for PPIs and SGIs */ > + if ( REG_RANK_NR(8, gicd_reg - GICD_ITARGETSR) == 0 ) > + return 1; This is handled by a different case of the switch statement. > + /* SPI delivery to secondary vcpus is unimplemented */ > + if ( REG_RANK_NR(8, gicd_reg - GICD_ITARGETSR) > 0 && > + *r != (1|1<<8|1<<16|1<<24) ) > + return 1; Given that itargets is (about to be, in the next patch) initialised to the same value isn't this entire hunk just a complicated way of saying "goto write_ignore"? (and if not please use goto write_ignore with the condition, since it is self documenting) And the REG_RANK_NR check here is unnecessary, because you checked for == 0 already (and as I say it's handled further up anyway). But -- doesn't your second patch actually make writing to ITARGETSR work? > vgic_lock_rank(v, rank); > if ( dabt.size == 2 ) > rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR)] = *r;
Hi Stefano, On 06/03/2014 05:07 PM, Stefano Stabellini wrote: > Ignore guest writes to GICD_ITARGETSR that set the target cpu to a cpu > other than cpu0 for SPIs. > > Also ignore guest writes to GICD_ITARGETSR for PPIs and SGIs as they can > only be delivered to the same cpu and that has already been configured > at initialization time. > > Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> > > --- > > Changes in v2: > - ignore writes to rank 0; > - don't print a warning for ignoring writes to GICD_ITARGETSR; > - add a comment in the code to remember that we don't implement writes > to GICD_ITARGETSR. > --- > xen/arch/arm/vgic.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index cb8df3a..1304b5e 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c > @@ -584,6 +584,13 @@ 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; > + /* only same vcpu delivery can be allowed for PPIs and SGIs */ > + if ( REG_RANK_NR(8, gicd_reg - GICD_ITARGETSR) == 0 ) > + return 1; This test is wrong. PPIs and SGIs are already a preset value (see vcpu_vgic_init). Futhermore, there is already a specific case for PPIs and SGIs (see few lines above). > + /* SPI delivery to secondary vcpus is unimplemented */ > + if ( REG_RANK_NR(8, gicd_reg - GICD_ITARGETSR) > 0 && > + *r != (1|1<<8|1<<16|1<<24) ) > + return 1; You don't handle byte-access here. Rather than testing the value, I think it's fine to return unconditionally. > vgic_lock_rank(v, rank); > if ( dabt.size == 2 ) > rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR)] = *r; > Regards,
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index cb8df3a..1304b5e 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -584,6 +584,13 @@ 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; + /* only same vcpu delivery can be allowed for PPIs and SGIs */ + if ( REG_RANK_NR(8, gicd_reg - GICD_ITARGETSR) == 0 ) + return 1; + /* SPI delivery to secondary vcpus is unimplemented */ + if ( REG_RANK_NR(8, gicd_reg - GICD_ITARGETSR) > 0 && + *r != (1|1<<8|1<<16|1<<24) ) + return 1; vgic_lock_rank(v, rank); if ( dabt.size == 2 ) rank->itargets[REG_RANK_INDEX(8, gicd_reg - GICD_ITARGETSR)] = *r;
Ignore guest writes to GICD_ITARGETSR that set the target cpu to a cpu other than cpu0 for SPIs. Also ignore guest writes to GICD_ITARGETSR for PPIs and SGIs as they can only be delivered to the same cpu and that has already been configured at initialization time. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> --- Changes in v2: - ignore writes to rank 0; - don't print a warning for ignoring writes to GICD_ITARGETSR; - add a comment in the code to remember that we don't implement writes to GICD_ITARGETSR. --- xen/arch/arm/vgic.c | 7 +++++++ 1 file changed, 7 insertions(+)