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

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

[Ayan Kumar Halder]

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@amd.com>
---
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`
+
+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
+currently unsupported by Xen.
+
+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;
     }
-- 
2.25.1

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

[Orzel, Michal]

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@amd.com>
> ---
> 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

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

[Ayan Kumar Halder]

Hi Michal,

On 09/07/2025 14:38, Orzel, Michal wrote:
>
> 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@amd.com>
>> ---
>> 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.

While writing this, I was thinking that this is an unexpected error. Can 
I name this

`CovGapExp~gicv3_lr~1`, so we can correlate this with the component ?

>
>> +
>> +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
Ack
>
>> +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.

I agree. Should I say ?

...... currently unsupported by GICv3 (Refer Arm IHI 0069H.bID041224, 
Sec 12.4.9 ICH_VTR_EL2).

Thus, we know eactly the source of truth. If Arm specs change in future 
and Xen is updated, then we can update with reference to the latest doc.

- Ayan

>
>> +
>> +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