Re: [RFC PATCH] xen/arm: Track coverage gap due to architecture limitations

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 09/07/2025 15:12, Ayan Kumar Halder wrote:
> There are a number of places where Xen triggers a BUG() due to 'impossible'
> conditions. One of these impossible condition is when gicv3_info.nr_lrs is
> equal to 0 or greater than 16.
> 
> Add a OFT marker in the code and link it to document explaining why this
> condition is impossible to be covered while running Xen on a platform. As a
> consequence, explain the architectural limitation that Xen relies on.
> 
> Signed-off-by: Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
> ---
> While running Xen with coverage, there will be some parts of code which cannot
> be covered as long as we run Xen on a supported platform. If we have link the
> code with the specific explanation, this will provide some context for the
> missing coverage. Also if the relevant part of the code is ever modified, the
> dev can update the explanation.
> 
>  docs/fusa/coverage_gaps.rst | 17 +++++++++++++++++
>  xen/arch/arm/gic-v3.c       |  4 ++++
>  2 files changed, 21 insertions(+)
>  create mode 100644 docs/fusa/coverage_gaps.rst
> 
> diff --git a/docs/fusa/coverage_gaps.rst b/docs/fusa/coverage_gaps.rst
> new file mode 100644
> index 0000000000..3cb877a8ee
> --- /dev/null
> +++ b/docs/fusa/coverage_gaps.rst
> @@ -0,0 +1,17 @@
> +
> +Coverage gap
> +============
> +
> +No support for zero or greater than 16 LRs
> +------------------------------------------
> +
> +`CovGapExp~unexp_err~1`
I think the tags need to be named better. 'unexp_err' does not mean a lot when
seen in a code.

> +
> +Description:
> +gicv3_info.nr_lrs is populated by reading ICH_VTR_EL2.ListRegs. The supported
> +number of list resgister is 1 - 16. Thus, any value outside of the range is
s/resgister/registers

> +currently unsupported by Xen.
Having a description written this way is misleading as it gives a false
impression that this is Xen limitation. It is not. ListRegs bitfield is 4 bits
width, therefore it's a GIC limit.

> +
> +Needs:
> + - CovGap
> +
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index cd3e1acf79..eca4d76f5a 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -109,6 +109,7 @@ static inline void gicv3_save_lrs(struct vcpu *v)
>      case 1:
>           v->arch.gic.v3.lr[0] = READ_SYSREG_LR(0);
>           break;
> +    /* [CovGap~~1->CovGapExp~unexp_err~1] */
>      default:
>           BUG();
>      }
> @@ -171,6 +172,7 @@ static inline void gicv3_restore_lrs(const struct vcpu *v)
>      case 1:
>          WRITE_SYSREG_LR(v->arch.gic.v3.lr[0], 0);
>          break;
> +    /* [CovGap~~1->CovGapExp~unexp_err~1] */
>      default:
>           BUG();
>      }
> @@ -196,6 +198,7 @@ static uint64_t gicv3_ich_read_lr(int lr)
>      case 13: return READ_SYSREG_LR(13);
>      case 14: return READ_SYSREG_LR(14);
>      case 15: return READ_SYSREG_LR(15);
> +    /* [CovGap~~1->CovGapExp~unexp_err~1] */
>      default:
>          BUG();
>      }
> @@ -253,6 +256,7 @@ static void gicv3_ich_write_lr(int lr, uint64_t val)
>      case 15:
>          WRITE_SYSREG_LR(val, 15);
>          break;
> +    /* [CovGap~~1->CovGapExp~unexp_err~1] */
>      default:
>          return;
>      }~Michal