[XEN PATCH v2 0/3] fix violations of MISRA C:2012 Rule 3.1

[XEN PATCH v2 0/3] fix violations of MISRA C:2012 Rule 3.1

[Nicola Vetrini]

Hi all,

This patch series is about the violations present in the Xen sources of
Rule 3.1 from MISRA C:2012, whose headline states:
"The character sequences '/*' and '//' shall not be used within a comment".

In the context of the effort to bring xen into compliance w.r.t.
MISRA C:2012, and Rule 3.1 being already approved for the project (as
evidenced by `docs/misra/rules.rst'), these violations need to be fixed.

Most of the violations are due to the presence of links to webpages within
C-style comment blocks, such as:

xen/arch/arm/include/asm/smccc.h:37.1-41.3
/*
 * This file provides common defines for ARM SMC Calling Convention as
 * specified in
 * http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html
*/

In this case, I propose to deviate all of these occurrences with a
project deviation to be captured by a tool configuration
(not included in any patch from this series).

There are, however, a few other violations that do not fall under this
category, which are the focus of the following set of patches. They either:

1. remove the nested '//' character sequence within a block comment;
2. remove the surrounding comment.

Thanks,
  Nicola

Nicola Vetrini (3):
  xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1
  xen/drivers/passthrough/arm/smmu-v3.c: fix violations of MISRA C:2012
    Rule 3.1
  xen: fix violations of MISRA C:2012 Rule 3.1

 xen/arch/arm/include/asm/arm32/flushtlb.h | 8 ++++----
 xen/arch/arm/include/asm/arm64/flushtlb.h | 8 ++++----
 xen/common/xmalloc_tlsf.c                 | 3 ---
 xen/drivers/passthrough/arm/smmu-v3.c     | 4 ++--
 xen/include/xen/atomic.h                  | 2 +-
 5 files changed, 11 insertions(+), 14 deletions(-)

-- 
2.34.1

[XEN PATCH v2 1/3] xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1

[Nicola Vetrini]

In the files `xen/arch/arm/include/asm/arm(32|64)/flushtlb.h' there are a
few occurrences of nested '//' character sequences inside C-style comment
blocks, which violate Rule 3.1. The patch aims to resolve those by removing
the nested comments.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com
---
Changes:
- Resending the patch with the right maintainers in CC.
Changes in V2:
- Split the patch into a series and reworked the fix.
- Apply the fix to the arm32 `flushtlb.h' file, for consistency
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/arch/arm/include/asm/arm32/flushtlb.h | 8 ++++----
 xen/arch/arm/include/asm/arm64/flushtlb.h | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/include/asm/arm32/flushtlb.h b/xen/arch/arm/include/asm/arm32/flushtlb.h
index 22ee3b317b..bcbeac590b 100644
--- a/xen/arch/arm/include/asm/arm32/flushtlb.h
+++ b/xen/arch/arm/include/asm/arm32/flushtlb.h
@@ -4,10 +4,10 @@
 /*
  * Every invalidation operation use the following patterns:
  *
- * DSB ISHST        // Ensure prior page-tables updates have completed
- * TLBI...          // Invalidate the TLB
- * DSB ISH          // Ensure the TLB invalidation has completed
- * ISB              // See explanation below
+ * DSB ISHST        Ensure prior page-tables updates have completed
+ * TLBI...          Invalidate the TLB
+ * DSB ISH          Ensure the TLB invalidation has completed
+ * ISB              See explanation below
  *
  * For Xen page-tables the ISB will discard any instructions fetched
  * from the old mappings.
diff --git a/xen/arch/arm/include/asm/arm64/flushtlb.h b/xen/arch/arm/include/asm/arm64/flushtlb.h
index 56c6fc763b..6066a2d703 100644
--- a/xen/arch/arm/include/asm/arm64/flushtlb.h
+++ b/xen/arch/arm/include/asm/arm64/flushtlb.h
@@ -4,10 +4,10 @@
 /*
  * Every invalidation operation use the following patterns:
  *
- * DSB ISHST        // Ensure prior page-tables updates have completed
- * TLBI...          // Invalidate the TLB
- * DSB ISH          // Ensure the TLB invalidation has completed
- * ISB              // See explanation below
+ * DSB ISHST        Ensure prior page-tables updates have completed
+ * TLBI...          Invalidate the TLB
+ * DSB ISH          Ensure the TLB invalidation has completed
+ * ISB              See explanation below
  *
  * ARM64_WORKAROUND_REPEAT_TLBI:
  * Modification of the translation table for a virtual address might lead to
-- 
2.34.1

Re: [XEN PATCH v2 1/3] xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1

[Julien Grall]

Hi,

On 19/06/2023 10:56, Nicola Vetrini wrote:
> In the files `xen/arch/arm/include/asm/arm(32|64)/flushtlb.h' there are a
> few occurrences of nested '//' character sequences inside C-style comment
> blocks, which violate Rule 3.1. The patch aims to resolve those by removing
> the nested comments.

As I wrote in 
https://lore.kernel.org/xen-devel/f3fc1848-68ca-37a1-add2-e100b4773190@xen.org/, 
I am against replacing '//' with nothing. I have proposed to use ';' 
because this is also a valid way to comment in assembly.

> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com

There is a missing '>' at the end of the list.

> ---
> Changes:
> - Resending the patch with the right maintainers in CC.
> Changes in V2:
> - Split the patch into a series and reworked the fix.
> - Apply the fix to the arm32 `flushtlb.h' file, for consistency
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
>   xen/arch/arm/include/asm/arm32/flushtlb.h | 8 ++++----
>   xen/arch/arm/include/asm/arm64/flushtlb.h | 8 ++++----
>   2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/include/asm/arm32/flushtlb.h b/xen/arch/arm/include/asm/arm32/flushtlb.h
> index 22ee3b317b..bcbeac590b 100644
> --- a/xen/arch/arm/include/asm/arm32/flushtlb.h
> +++ b/xen/arch/arm/include/asm/arm32/flushtlb.h
> @@ -4,10 +4,10 @@
>   /*
>    * Every invalidation operation use the following patterns:
>    *
> - * DSB ISHST        // Ensure prior page-tables updates have completed
> - * TLBI...          // Invalidate the TLB
> - * DSB ISH          // Ensure the TLB invalidation has completed
> - * ISB              // See explanation below
> + * DSB ISHST        Ensure prior page-tables updates have completed
> + * TLBI...          Invalidate the TLB
> + * DSB ISH          Ensure the TLB invalidation has completed
> + * ISB              See explanation below
>    *
>    * For Xen page-tables the ISB will discard any instructions fetched
>    * from the old mappings.
> diff --git a/xen/arch/arm/include/asm/arm64/flushtlb.h b/xen/arch/arm/include/asm/arm64/flushtlb.h
> index 56c6fc763b..6066a2d703 100644
> --- a/xen/arch/arm/include/asm/arm64/flushtlb.h
> +++ b/xen/arch/arm/include/asm/arm64/flushtlb.h
> @@ -4,10 +4,10 @@
>   /*
>    * Every invalidation operation use the following patterns:
>    *
> - * DSB ISHST        // Ensure prior page-tables updates have completed
> - * TLBI...          // Invalidate the TLB
> - * DSB ISH          // Ensure the TLB invalidation has completed
> - * ISB              // See explanation below
> + * DSB ISHST        Ensure prior page-tables updates have completed
> + * TLBI...          Invalidate the TLB
> + * DSB ISH          Ensure the TLB invalidation has completed
> + * ISB              See explanation below
>    *
>    * ARM64_WORKAROUND_REPEAT_TLBI:
>    * Modification of the translation table for a virtual address might lead to

Cheers,

-- 
Julien Grall

Re: [XEN PATCH v2 1/3] xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1

[Jan Beulich]

On 19.06.2023 12:01, Julien Grall wrote:
> On 19/06/2023 10:56, Nicola Vetrini wrote:
>> In the files `xen/arch/arm/include/asm/arm(32|64)/flushtlb.h' there are a
>> few occurrences of nested '//' character sequences inside C-style comment
>> blocks, which violate Rule 3.1. The patch aims to resolve those by removing
>> the nested comments.
> 
> As I wrote in 
> https://lore.kernel.org/xen-devel/f3fc1848-68ca-37a1-add2-e100b4773190@xen.org/, 
> I am against replacing '//' with nothing. I have proposed to use ';' 
> because this is also a valid way to comment in assembly.

Are you sure about this? For gas most targets use ; as a statement separator,
not as a comment character. Afaics arm-* and aarch64-* are no exception there.

Jan

Re: [XEN PATCH v2 1/3] xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1

[Julien Grall]

Hi,

On 19/06/2023 11:25, Jan Beulich wrote:
> On 19.06.2023 12:01, Julien Grall wrote:
>> On 19/06/2023 10:56, Nicola Vetrini wrote:
>>> In the files `xen/arch/arm/include/asm/arm(32|64)/flushtlb.h' there are a
>>> few occurrences of nested '//' character sequences inside C-style comment
>>> blocks, which violate Rule 3.1. The patch aims to resolve those by removing
>>> the nested comments.
>>
>> As I wrote in
>> https://lore.kernel.org/xen-devel/f3fc1848-68ca-37a1-add2-e100b4773190@xen.org/,
>> I am against replacing '//' with nothing. I have proposed to use ';'
>> because this is also a valid way to comment in assembly.
> 
> Are you sure about this? For gas most targets use ; as a statement separator,
> not as a comment character. Afaics arm-* and aarch64-* are no exception there.

GAS will not accept it. But the Arm compiler will [1]. This is good 
enough for me because I want to have a separator between the instruction 
and the comment.

Cheers,

[1] 
https://developer.arm.com/documentation/dui0473/m/structure-of-assembly-language-modules/syntax-of-source-lines-in-assembly-language

-- 
Julien Grall

Re: [XEN PATCH v2 1/3] xen/arch/arm: fix violations of MISRA C:2012 Rule 3.1

[Nicola Vetrini]

Hi,

On 19/06/23 12:29, Julien Grall wrote:
> Hi,
> 
> On 19/06/2023 11:25, Jan Beulich wrote:
>> On 19.06.2023 12:01, Julien Grall wrote:
>>> On 19/06/2023 10:56, Nicola Vetrini wrote:
>>>> In the files `xen/arch/arm/include/asm/arm(32|64)/flushtlb.h' there 
>>>> are a
>>>> few occurrences of nested '//' character sequences inside C-style 
>>>> comment
>>>> blocks, which violate Rule 3.1. The patch aims to resolve those by 
>>>> removing
>>>> the nested comments.
>>>
>>> As I wrote in
>>> https://lore.kernel.org/xen-devel/f3fc1848-68ca-37a1-add2-e100b4773190@xen.org/,
>>> I am against replacing '//' with nothing. I have proposed to use ';'
>>> because this is also a valid way to comment in assembly.
>>
>> Are you sure about this? For gas most targets use ; as a statement 
>> separator,
>> not as a comment character. Afaics arm-* and aarch64-* are no 
>> exception there.
> 
> GAS will not accept it. But the Arm compiler will [1]. This is good 
> enough for me because I want to have a separator between the instruction 
> and the comment.
> 
> Cheers,
> 
> [1] 
> https://developer.arm.com/documentation/dui0473/m/structure-of-assembly-language-modules/syntax-of-source-lines-in-assembly-language
> 

If no one has any objection about this I'll add the ';' comment delimiter.

Regards,

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

[XEN PATCH v2 2/3] xen/drivers/passthrough/arm/smmu-v3.c: fix violations of MISRA C:2012 Rule 3.1

[Nicola Vetrini]

In the file `xen/drivers/passthrough/arm/smmu-v3.c' there are a few occurrences
of nested '//' character sequences inside C-style comment blocks, which violate
Rule 3.1. The patch aims to resolve those by removing the nested comments.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com
Changes:
- Resending the patch with the right maintainers in CC.
Changes in V2:
- Split the patch into a series and reworked the fix.
- Apply the fix to the arm32 `flushtlb.h' file, for consistency
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/drivers/passthrough/arm/smmu-v3.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
index 720aa69ff2..f410863e10 100644
--- a/xen/drivers/passthrough/arm/smmu-v3.c
+++ b/xen/drivers/passthrough/arm/smmu-v3.c
@@ -1047,10 +1047,10 @@ static int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain,
 	 * before we read 'nr_ats_masters' in case of a concurrent call to
 	 * arm_smmu_enable_ats():
 	 *
-	 *	// unmap()			// arm_smmu_enable_ats()
+	 *	unmap()				arm_smmu_enable_ats()
 	 *	TLBI+SYNC			atomic_inc(&nr_ats_masters);
 	 *	smp_mb();			[...]
-	 *	atomic_read(&nr_ats_masters);	pci_enable_ats() // writel()
+	 *	atomic_read(&nr_ats_masters);	pci_enable_ats() (i.e. writel())
 	 *
 	 * Ensures that we always see the incremented 'nr_ats_masters' count if
 	 * ATS was enabled at the PCI device before completion of the TLBI.
-- 
2.34.1

Re: [XEN PATCH v2 2/3] xen/drivers/passthrough/arm/smmu-v3.c: fix violations of MISRA C:2012 Rule 3.1

[Julien Grall]

Hi,

On 19/06/2023 10:56, Nicola Vetrini wrote:
> In the file `xen/drivers/passthrough/arm/smmu-v3.c' there are a few occurrences
> of nested '//' character sequences inside C-style comment blocks, which violate
> Rule 3.1. The patch aims to resolve those by removing the nested comments.

I think it is important to understand/explain what was the intention of 
the // before removing them because, IMHO, the new approach doesn't 
convey the same information. Before...

> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com
> Changes:
> - Resending the patch with the right maintainers in CC.
> Changes in V2:
> - Split the patch into a series and reworked the fix.
> - Apply the fix to the arm32 `flushtlb.h' file, for consistency
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
> ---
>   xen/drivers/passthrough/arm/smmu-v3.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c b/xen/drivers/passthrough/arm/smmu-v3.c
> index 720aa69ff2..f410863e10 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -1047,10 +1047,10 @@ static int arm_smmu_atc_inv_domain(struct arm_smmu_domain *smmu_domain,
>   	 * before we read 'nr_ats_masters' in case of a concurrent call to
>   	 * arm_smmu_enable_ats():
>   	 *
> -	 *	// unmap()			// arm_smmu_enable_ats()
> +	 *	unmap()				arm_smmu_enable_ats()

... with the // it would be clearer that the code below belongs to each 
function. But now, it leads to think there are a call to 'unmap' which 
it then followed by TLBI+SYNC.

In this case, I would propose to use --- <function> ---

>   	 *	TLBI+SYNC			atomic_inc(&nr_ats_masters);
>   	 *	smp_mb();			[...]
> -	 *	atomic_read(&nr_ats_masters);	pci_enable_ats() // writel()
> +	 *	atomic_read(&nr_ats_masters);	pci_enable_ats() (i.e. writel())

NIT: I think 'see' would be better than 'i.e.' because I read it as 
pci_enable_ats() is a simple writel().

>   	 *
>   	 * Ensures that we always see the incremented 'nr_ats_masters' count if
>   	 * ATS was enabled at the PCI device before completion of the TLBI.

Cheers,

-- 
Julien Grall

Re: [XEN PATCH v2 2/3] xen/drivers/passthrough/arm/smmu-v3.c: fix violations of MISRA C:2012 Rule 3.1

[Nicola Vetrini]

On 19/06/23 12:10, Julien Grall wrote:
> Hi,
> 
> On 19/06/2023 10:56, Nicola Vetrini wrote:
>> In the file `xen/drivers/passthrough/arm/smmu-v3.c' there are a few 
>> occurrences
>> of nested '//' character sequences inside C-style comment blocks, 
>> which violate
>> Rule 3.1. The patch aims to resolve those by removing the nested 
>> comments.
> 
> I think it is important to understand/explain what was the intention of 
> the // before removing them because, IMHO, the new approach doesn't 
> convey the same information. Before...
> 
>>
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com
>> Changes:
>> - Resending the patch with the right maintainers in CC.
>> Changes in V2:
>> - Split the patch into a series and reworked the fix.
>> - Apply the fix to the arm32 `flushtlb.h' file, for consistency
>> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
>> ---
>>   xen/drivers/passthrough/arm/smmu-v3.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
>> b/xen/drivers/passthrough/arm/smmu-v3.c
>> index 720aa69ff2..f410863e10 100644
>> --- a/xen/drivers/passthrough/arm/smmu-v3.c
>> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
>> @@ -1047,10 +1047,10 @@ static int arm_smmu_atc_inv_domain(struct 
>> arm_smmu_domain *smmu_domain,
>>        * before we read 'nr_ats_masters' in case of a concurrent call to
>>        * arm_smmu_enable_ats():
>>        *
>> -     *    // unmap()            // arm_smmu_enable_ats()
>> +     *    unmap()                arm_smmu_enable_ats()
> 
> ... with the // it would be clearer that the code below belongs to each 
> function. But now, it leads to think there are a call to 'unmap' which 
> it then followed by TLBI+SYNC.
> 
> In this case, I would propose to use --- <function> ---

It seems a good suggestion to me.

> 
>>        *    TLBI+SYNC            atomic_inc(&nr_ats_masters);
>>        *    smp_mb();            [...]
>> -     *    atomic_read(&nr_ats_masters);    pci_enable_ats() // writel()
>> +     *    atomic_read(&nr_ats_masters);    pci_enable_ats() (i.e. 
>> writel())
> 
> NIT: I think 'see' would be better than 'i.e.' because I read it as 
> pci_enable_ats() is a simple writel().

Ok.

> 
>>        *
>>        * Ensures that we always see the incremented 'nr_ats_masters' 
>> count if
>>        * ATS was enabled at the PCI device before completion of the TLBI.
> 
> Cheers,
> 

Regards,

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

[XEN PATCH v2 3/3] xen: fix violations of MISRA C:2012 Rule 3.1

[Nicola Vetrini]

In the files modified by this patch there are a few occurrences of nested '//'
character sequences inside C-style comment blocks, which violate Rule 3.1.
The patch aims to resolve those by removing the nested comments.

In the file `xen/common/xmalloc_tlsf.c' the comment has been deleted,
following the suggestion of a review comment.

In the file `xen/include/xen/atomic.h' the nested comment has been removed,
since the code sample is already explained by the preceding comment.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com
Changes:
- Resending the patch with the right maintainers in CC.
Changes in V2:
- Split the patch into a series and reworked the fix.
- Apply the fix to the arm32 `flushtlb.h' file, for consistency
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 xen/common/xmalloc_tlsf.c | 3 ---
 xen/include/xen/atomic.h  | 2 +-
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/xen/common/xmalloc_tlsf.c b/xen/common/xmalloc_tlsf.c
index 75bdf18c4e..4f9f60a39d 100644
--- a/xen/common/xmalloc_tlsf.c
+++ b/xen/common/xmalloc_tlsf.c
@@ -140,9 +140,6 @@ static inline void MAPPING_SEARCH(unsigned long *r, int *fl, int *sl)
         *fl = flsl(*r) - 1;
         *sl = (*r >> (*fl - MAX_LOG2_SLI)) - MAX_SLI;
         *fl -= FLI_OFFSET;
-        /*if ((*fl -= FLI_OFFSET) < 0) // FL will be always >0!
-         *fl = *sl = 0;
-         */
         *r &= ~t;
     }
 }
diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
index 529213ebbb..fa750a18ae 100644
--- a/xen/include/xen/atomic.h
+++ b/xen/include/xen/atomic.h
@@ -78,7 +78,7 @@ static inline void _atomic_set(atomic_t *v, int i);
  *      int old = atomic_read(&v);
  *      int new = old + 1;
  *      if ( likely(old == atomic_cmpxchg(&v, old, new)) )
- *          break; // success!
+ *          break;
  *  }
  */
 static inline int atomic_cmpxchg(atomic_t *v, int old, int new);
-- 
2.34.1

Re: [XEN PATCH v2 3/3] xen: fix violations of MISRA C:2012 Rule 3.1

[Jan Beulich]

On 19.06.2023 11:56, Nicola Vetrini wrote:
> --- a/xen/common/xmalloc_tlsf.c
> +++ b/xen/common/xmalloc_tlsf.c
> @@ -140,9 +140,6 @@ static inline void MAPPING_SEARCH(unsigned long *r, int *fl, int *sl)
>          *fl = flsl(*r) - 1;
>          *sl = (*r >> (*fl - MAX_LOG2_SLI)) - MAX_SLI;
>          *fl -= FLI_OFFSET;
> -        /*if ((*fl -= FLI_OFFSET) < 0) // FL will be always >0!
> -         *fl = *sl = 0;
> -         */
>          *r &= ~t;
>      }
>  }

As indicated before, I don't think simply dropping the commented out code
is appropriate here. Personally I'd prefer if it was kept (using #if/#else),
but I'd also be okay with replacing it by a respective assertion. That said,
if other maintainers think this is the way to go, then I don't mean to
stand in the way.

Jan

Re: [XEN PATCH v2 3/3] xen: fix violations of MISRA C:2012 Rule 3.1

[Nicola Vetrini]

On 19/06/23 12:29, Jan Beulich wrote:
> On 19.06.2023 11:56, Nicola Vetrini wrote:
>> --- a/xen/common/xmalloc_tlsf.c
>> +++ b/xen/common/xmalloc_tlsf.c
>> @@ -140,9 +140,6 @@ static inline void MAPPING_SEARCH(unsigned long *r, int *fl, int *sl)
>>           *fl = flsl(*r) - 1;
>>           *sl = (*r >> (*fl - MAX_LOG2_SLI)) - MAX_SLI;
>>           *fl -= FLI_OFFSET;
>> -        /*if ((*fl -= FLI_OFFSET) < 0) // FL will be always >0!
>> -         *fl = *sl = 0;
>> -         */
>>           *r &= ~t;
>>       }
>>   }
> 
> As indicated before, I don't think simply dropping the commented out code
> is appropriate here. Personally I'd prefer if it was kept (using #if/#else),
> but I'd also be okay with replacing it by a respective assertion. That said,
> if other maintainers think this is the way to go, then I don't mean to
> stand in the way.
> 

As Andrew Cooper suggested in the previous patch revision 
(https://lore.kernel.org/xen-devel/6bac57d5-c30e-f763-3abe-b3f335f366f7@suse.com/T/#m5722285215bb30d7f1202b9921e2c92d5ea98d6a), 
I removed the commented-out code, since it contains unused logic, but I 
would be okay with replacing it with an assertion, if you think it's better.

Regards,

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)