| Message ID | 20240902100355.3032079-3-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ARM: Cleanup following bitops improvements | expand |
On 02/09/2024 12:03, Andrew Cooper wrote: > > > The current expression hits UB with 31 LRs (shifting into the sign bit), and > malfunctions with 32 LRs (shifting beyond the range of int). Swapping 1 for > 1ULL fixes some of these, but still malfunctions at 64 LRs which is the > architectural limit. > > Instead, shift -1ULL right in order to create the mask. > > Fixes: 596f885a3202 ("xen/arm: set GICH_HCR_UIE if all the LRs are in use") > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Michal Orzel <michal.orzel@amd.com> > --- > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Julien Grall <julien@xen.org> > CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> > CC: Bertrand Marquis <bertrand.marquis@arm.com> > CC: Michal Orzel <michal.orzel@amd.com> > > Found by code inspection while doing bitops work. I don't even know if > there's a platform that really has 31 LRs, but the rest of Xen's code is > written with the expectation that there may be 64. So, for GICv2 the limit is 64 and for GICv3 the limit is 16. This made me realize we need to fix the mask for GICv3 (ICH_VTR_NRLRGS) from 0x3f to 0xf. I'll send a patch. > --- > xen/arch/arm/gic-vgic.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c > index 9aa245a36d98..3f14aab2efc7 100644 > --- a/xen/arch/arm/gic-vgic.c > +++ b/xen/arch/arm/gic-vgic.c > @@ -16,7 +16,8 @@ > #include <asm/gic.h> > #include <asm/vgic.h> > > -#define lr_all_full() (this_cpu(lr_mask) == ((1 << gic_get_nr_lrs()) - 1)) > +#define lr_all_full() \ > + (this_cpu(lr_mask) == (-1ULL >> (64 - gic_get_nr_lrs()))) > > #undef GIC_DEBUG > > -- > 2.39.2 > ~Michal
diff --git a/xen/arch/arm/gic-vgic.c b/xen/arch/arm/gic-vgic.c index 9aa245a36d98..3f14aab2efc7 100644 --- a/xen/arch/arm/gic-vgic.c +++ b/xen/arch/arm/gic-vgic.c @@ -16,7 +16,8 @@ #include <asm/gic.h> #include <asm/vgic.h> -#define lr_all_full() (this_cpu(lr_mask) == ((1 << gic_get_nr_lrs()) - 1)) +#define lr_all_full() \ + (this_cpu(lr_mask) == (-1ULL >> (64 - gic_get_nr_lrs()))) #undef GIC_DEBUG
The current expression hits UB with 31 LRs (shifting into the sign bit), and malfunctions with 32 LRs (shifting beyond the range of int). Swapping 1 for 1ULL fixes some of these, but still malfunctions at 64 LRs which is the architectural limit. Instead, shift -1ULL right in order to create the mask. Fixes: 596f885a3202 ("xen/arm: set GICH_HCR_UIE if all the LRs are in use") Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien@xen.org> CC: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com> CC: Bertrand Marquis <bertrand.marquis@arm.com> CC: Michal Orzel <michal.orzel@amd.com> Found by code inspection while doing bitops work. I don't even know if there's a platform that really has 31 LRs, but the rest of Xen's code is written with the expectation that there may be 64. --- xen/arch/arm/gic-vgic.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)