[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 2/3] xen/arm: vgic: Use 'unsigned int' rather than 'int' whenever it is possible
On Thu, 17 Aug 2023, Julien Grall wrote: > From: Julien Grall <jgrall@xxxxxxxxxx> > > Switch to unsigned int for the return/parameters of the following > functions: > * REG_RANK_NR(): 'b' (number of bits) and the return is always positive. > 'n' doesn't need to be size specific. > * vgic_rank_offset(): 'b' (number of bits), 'n' (register index), > 's' (size of the access) are always positive. > * vgic_{enable, disable}_irqs(): 'n' (rank index) is always positive > * vgic_get_virq_type(): 'n' (rank index) and 'index' (register > index) are always positive. > > Take the opportunity to propogate the unsignedness to the local > variable used for the arguments. > > This will remove some of the warning reported by GCC 12.2.1 when > passing the flags -Wsign-conversion/-Wconversion. > > Signed-off-by: Julien Grall <jgrall@xxxxxxxxxx> > --- > xen/arch/arm/include/asm/vgic.h | 11 +++++++---- > xen/arch/arm/vgic-v2.c | 12 ++++++++++-- > xen/arch/arm/vgic.c | 21 ++++++++++++--------- > 3 files changed, 29 insertions(+), 15 deletions(-) > > diff --git a/xen/arch/arm/include/asm/vgic.h b/xen/arch/arm/include/asm/vgic.h > index 6901a05c0669..922779ce146a 100644 > --- a/xen/arch/arm/include/asm/vgic.h > +++ b/xen/arch/arm/include/asm/vgic.h > @@ -252,7 +252,7 @@ struct vgic_ops { > * Rank containing GICD_<FOO><n> for GICD_<FOO> with > * <b>-bits-per-interrupt > */ > -static inline int REG_RANK_NR(int b, uint32_t n) > +static inline unsigned int REG_RANK_NR(unsigned int b, unsigned int n) > { > switch ( b ) > { > @@ -297,10 +297,13 @@ extern void gic_remove_from_lr_pending(struct vcpu *v, > struct pending_irq *p); > extern void vgic_init_pending_irq(struct pending_irq *p, unsigned int virq); > extern struct pending_irq *irq_to_pending(struct vcpu *v, unsigned int irq); > extern struct pending_irq *spi_to_pending(struct domain *d, unsigned int > irq); > -extern struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v, int b, int n, > int s); > +extern struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v, > + unsigned int b, > + unsigned int n, > + unsigned int s); > extern struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq); > -extern void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n); > -extern void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n); > +extern void vgic_disable_irqs(struct vcpu *v, uint32_t r, unsigned int n); > +extern void vgic_enable_irqs(struct vcpu *v, uint32_t r, unsigned int n); > extern void vgic_set_irqs_pending(struct vcpu *v, uint32_t r, > unsigned int rank); > extern void register_vgic_ops(struct domain *d, const struct vgic_ops *ops); > diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c > index 2a2eda2e6f4c..0aa10fff0f10 100644 > --- a/xen/arch/arm/vgic-v2.c > +++ b/xen/arch/arm/vgic-v2.c > @@ -161,7 +161,11 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, > mmio_info_t *info, > { > struct hsr_dabt dabt = info->dabt; > struct vgic_irq_rank *rank; > - int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase); > + /* > + * gpa/dbase are paddr_t which size may be higher than 32-bit. Yet > + * the difference will always be smaller than 32-bit. > + */ > + unsigned int gicd_reg = info->gpa - v->domain->arch.vgic.dbase; > unsigned long flags; > > perfc_incr(vgicd_reads); > @@ -403,7 +407,11 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, > mmio_info_t *info, > { > struct hsr_dabt dabt = info->dabt; > struct vgic_irq_rank *rank; > - int gicd_reg = (int)(info->gpa - v->domain->arch.vgic.dbase); > + /* > + * gpa/dbase are paddr_t which size may be higher than 32-bit. Yet > + * the difference will always be smaller than 32-bit. > + */ > + unsigned int gicd_reg = info->gpa - v->domain->arch.vgic.dbase; > uint32_t tr; > unsigned long flags; > > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index afcac791fe4b..269b804974e0 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c > @@ -24,7 +24,8 @@ > #include <asm/gic.h> > #include <asm/vgic.h> > > -static inline struct vgic_irq_rank *vgic_get_rank(struct vcpu *v, int rank) > +static inline struct vgic_irq_rank *vgic_get_rank(struct vcpu *v, > + unsigned int rank) > { > if ( rank == 0 ) > return v->arch.vgic.private_irqs; > @@ -38,17 +39,17 @@ static inline struct vgic_irq_rank *vgic_get_rank(struct > vcpu *v, int rank) > * Returns rank corresponding to a GICD_<FOO><n> register for > * GICD_<FOO> with <b>-bits-per-interrupt. > */ > -struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v, int b, int n, > - int s) > +struct vgic_irq_rank *vgic_rank_offset(struct vcpu *v, unsigned int b, > + unsigned int n, unsigned int s) > { > - int rank = REG_RANK_NR(b, (n >> s)); > + unsigned int rank = REG_RANK_NR(b, (n >> s)); > > return vgic_get_rank(v, rank); > } > > struct vgic_irq_rank *vgic_rank_irq(struct vcpu *v, unsigned int irq) > { > - int rank = irq/32; > + unsigned int rank = irq / 32; > > return vgic_get_rank(v, rank); > } > @@ -324,14 +325,14 @@ void arch_move_irqs(struct vcpu *v) > } > } > > -void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n) > +void vgic_disable_irqs(struct vcpu *v, uint32_t r, unsigned int n) > { > const unsigned long mask = r; > struct pending_irq *p; > struct irq_desc *desc; > unsigned int irq; > unsigned long flags; > - int i = 0; > + unsigned int i = 0; > struct vcpu *v_target; > > /* LPIs will never be disabled via this function. */ > @@ -361,7 +362,9 @@ void vgic_disable_irqs(struct vcpu *v, uint32_t r, int n) > #define VGIC_ICFG_MASK(intr) (1U << ((2 * ((intr) % 16)) + 1)) > > /* The function should be called with the rank lock taken */ > -static inline unsigned int vgic_get_virq_type(struct vcpu *v, int n, int > index) > +static inline unsigned int vgic_get_virq_type(struct vcpu *v, > + unsigned int n, > + unsigned int index) > { > struct vgic_irq_rank *r = vgic_get_rank(v, n); > uint32_t tr = r->icfg[index >> 4]; > @@ -374,7 +377,7 @@ static inline unsigned int vgic_get_virq_type(struct vcpu > *v, int n, int index) > return IRQ_TYPE_LEVEL_HIGH; > } > > -void vgic_enable_irqs(struct vcpu *v, uint32_t r, int n) > +void vgic_enable_irqs(struct vcpu *v, uint32_t r, unsigned int n) > { > const unsigned long mask = r; > struct pending_irq *p; It looks like you could also switch 'i' to unsigned int in vgic_enable_irqs like you did in vgic_disable_irqs You could do that on commit: Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |