From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
The xen sources contain violations of MISRA C:2012 Rule 14.4 whose
headline states:
"The controlling expression of an if statement and the controlling
expression of an iteration-statement shall have essentially Boolean type".
Add comparisons to avoid using enum constants as controlling expressions
to comply with Rule 14.4.
No functional change.
Signed-off-by: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
---
xen/arch/x86/hpet.c | 6 +++---
xen/arch/x86/msi.c | 4 ++--
xen/arch/x86/x86_emulate/x86_emulate.c | 8 ++++----
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index 7be26c6a9b..d1ddc8ddf6 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -279,7 +279,7 @@ static int hpet_msi_write(struct hpet_event_channel *ch, struct msi_msg *msg)
{
ch->msi.msg = *msg;
- if ( iommu_intremap )
+ if ( iommu_intremap != iommu_intremap_off )
{
int rc = iommu_update_ire_from_msi(&ch->msi, msg);
@@ -353,7 +353,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
u32 cfg = hpet_read32(HPET_Tn_CFG(ch->idx));
irq_desc_t *desc = irq_to_desc(ch->msi.irq);
- if ( iommu_intremap )
+ if ( iommu_intremap != iommu_intremap_off )
{
ch->msi.hpet_id = hpet_blockid;
ret = iommu_setup_hpet_msi(&ch->msi);
@@ -372,7 +372,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
ret = __hpet_setup_msi_irq(desc);
if ( ret < 0 )
{
- if ( iommu_intremap )
+ if ( iommu_intremap != iommu_intremap_off )
iommu_update_ire_from_msi(&ch->msi, NULL);
return ret;
}
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index 7f8e794254..72dce2e4ab 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -189,7 +189,7 @@ static int write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
{
entry->msg = *msg;
- if ( iommu_intremap )
+ if ( iommu_intremap != iommu_intremap_off )
{
int rc;
@@ -555,7 +555,7 @@ int msi_free_irq(struct msi_desc *entry)
destroy_irq(entry[nr].irq);
/* Free the unused IRTE if intr remap enabled */
- if ( iommu_intremap )
+ if ( iommu_intremap != iommu_intremap_off )
iommu_update_ire_from_msi(entry + nr, NULL);
}
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index cf780da501..00b7365ed3 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1320,7 +1320,7 @@ x86_emulate(
ea.bytes = 2;
goto srcmem_common;
case SrcMem:
- if ( state->simd_size )
+ if ( state->simd_size != simd_none )
break;
ea.bytes = (d & ByteOp) ? 1 : op_bytes;
srcmem_common:
@@ -1460,7 +1460,7 @@ x86_emulate(
/* Becomes a normal DstMem operation from here on. */
case DstMem:
generate_exception_if(ea.type == OP_MEM && evex.z, X86_EXC_UD);
- if ( state->simd_size )
+ if ( state->simd_size != simd_none )
{
generate_exception_if(lock_prefix, X86_EXC_UD);
break;
@@ -8176,7 +8176,7 @@ x86_emulate(
goto done;
}
- if ( state->rmw )
+ if ( state->rmw != rmw_NONE )
{
ea.val = src.val;
op_bytes = dst.bytes;
@@ -8205,7 +8205,7 @@ x86_emulate(
dst.type = OP_NONE;
}
- else if ( state->simd_size )
+ else if ( state->simd_size != simd_none )
{
generate_exception_if(!op_bytes, X86_EXC_UD);
generate_exception_if((vex.opcx && (d & TwoOp) &&
--
2.40.0On Thu, 7 Dec 2023, Simone Ballarin wrote: > From: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com> > > The xen sources contain violations of MISRA C:2012 Rule 14.4 whose > headline states: > "The controlling expression of an if statement and the controlling > expression of an iteration-statement shall have essentially Boolean type". > > Add comparisons to avoid using enum constants as controlling expressions > to comply with Rule 14.4. > No functional change. > > Signed-off-by: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com> > Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com> I think it would have been better to put all the iommu_intremap changed in the same patch (patch #1). But anyway: Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
On 07.12.2023 10:48, Simone Ballarin wrote:
> --- a/xen/arch/x86/hpet.c
> +++ b/xen/arch/x86/hpet.c
> @@ -279,7 +279,7 @@ static int hpet_msi_write(struct hpet_event_channel *ch, struct msi_msg *msg)
> {
> ch->msi.msg = *msg;
>
> - if ( iommu_intremap )
> + if ( iommu_intremap != iommu_intremap_off )
> {
> int rc = iommu_update_ire_from_msi(&ch->msi, msg);
>
> @@ -353,7 +353,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
> u32 cfg = hpet_read32(HPET_Tn_CFG(ch->idx));
> irq_desc_t *desc = irq_to_desc(ch->msi.irq);
>
> - if ( iommu_intremap )
> + if ( iommu_intremap != iommu_intremap_off )
> {
> ch->msi.hpet_id = hpet_blockid;
> ret = iommu_setup_hpet_msi(&ch->msi);
> @@ -372,7 +372,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
> ret = __hpet_setup_msi_irq(desc);
> if ( ret < 0 )
> {
> - if ( iommu_intremap )
> + if ( iommu_intremap != iommu_intremap_off )
> iommu_update_ire_from_msi(&ch->msi, NULL);
> return ret;
> }
> diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
> index 7f8e794254..72dce2e4ab 100644
> --- a/xen/arch/x86/msi.c
> +++ b/xen/arch/x86/msi.c
> @@ -189,7 +189,7 @@ static int write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
> {
> entry->msg = *msg;
>
> - if ( iommu_intremap )
> + if ( iommu_intremap != iommu_intremap_off )
> {
> int rc;
>
> @@ -555,7 +555,7 @@ int msi_free_irq(struct msi_desc *entry)
> destroy_irq(entry[nr].irq);
>
> /* Free the unused IRTE if intr remap enabled */
> - if ( iommu_intremap )
> + if ( iommu_intremap != iommu_intremap_off )
> iommu_update_ire_from_msi(entry + nr, NULL);
> }
>
All of this would logically be part of patch 1. Is there a particular reason
why it wasn't done right there?
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -1320,7 +1320,7 @@ x86_emulate(
> ea.bytes = 2;
> goto srcmem_common;
> case SrcMem:
> - if ( state->simd_size )
> + if ( state->simd_size != simd_none )
> break;
> ea.bytes = (d & ByteOp) ? 1 : op_bytes;
> srcmem_common:
> @@ -1460,7 +1460,7 @@ x86_emulate(
> /* Becomes a normal DstMem operation from here on. */
> case DstMem:
> generate_exception_if(ea.type == OP_MEM && evex.z, X86_EXC_UD);
> - if ( state->simd_size )
> + if ( state->simd_size != simd_none )
> {
> generate_exception_if(lock_prefix, X86_EXC_UD);
> break;
> @@ -8176,7 +8176,7 @@ x86_emulate(
> goto done;
> }
>
> - if ( state->rmw )
> + if ( state->rmw != rmw_NONE )
> {
> ea.val = src.val;
> op_bytes = dst.bytes;
> @@ -8205,7 +8205,7 @@ x86_emulate(
>
> dst.type = OP_NONE;
> }
> - else if ( state->simd_size )
> + else if ( state->simd_size != simd_none )
> {
> generate_exception_if(!op_bytes, X86_EXC_UD);
> generate_exception_if((vex.opcx && (d & TwoOp) &&
I'd be (somewhat reluctantly) okay with ack-ing this part of the patch.
Jan
On 07/12/23 11:54, Jan Beulich wrote:
> On 07.12.2023 10:48, Simone Ballarin wrote:
>> --- a/xen/arch/x86/hpet.c
>> +++ b/xen/arch/x86/hpet.c
>> @@ -279,7 +279,7 @@ static int hpet_msi_write(struct hpet_event_channel *ch, struct msi_msg *msg)
>> {
>> ch->msi.msg = *msg;
>>
>> - if ( iommu_intremap )
>> + if ( iommu_intremap != iommu_intremap_off )
>> {
>> int rc = iommu_update_ire_from_msi(&ch->msi, msg);
>>
>> @@ -353,7 +353,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
>> u32 cfg = hpet_read32(HPET_Tn_CFG(ch->idx));
>> irq_desc_t *desc = irq_to_desc(ch->msi.irq);
>>
>> - if ( iommu_intremap )
>> + if ( iommu_intremap != iommu_intremap_off )
>> {
>> ch->msi.hpet_id = hpet_blockid;
>> ret = iommu_setup_hpet_msi(&ch->msi);
>> @@ -372,7 +372,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
>> ret = __hpet_setup_msi_irq(desc);
>> if ( ret < 0 )
>> {
>> - if ( iommu_intremap )
>> + if ( iommu_intremap != iommu_intremap_off )
>> iommu_update_ire_from_msi(&ch->msi, NULL);
>> return ret;
>> }
>> diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
>> index 7f8e794254..72dce2e4ab 100644
>> --- a/xen/arch/x86/msi.c
>> +++ b/xen/arch/x86/msi.c
>> @@ -189,7 +189,7 @@ static int write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
>> {
>> entry->msg = *msg;
>>
>> - if ( iommu_intremap )
>> + if ( iommu_intremap != iommu_intremap_off )
>> {
>> int rc;
>>
>> @@ -555,7 +555,7 @@ int msi_free_irq(struct msi_desc *entry)
>> destroy_irq(entry[nr].irq);
>>
>> /* Free the unused IRTE if intr remap enabled */
>> - if ( iommu_intremap )
>> + if ( iommu_intremap != iommu_intremap_off )
>> iommu_update_ire_from_msi(entry + nr, NULL);
>> }
>>
>
> All of this would logically be part of patch 1. Is there a particular reason
> why it wasn't done right there?
These changes and the ones in patch 1 are related, but still remain
independent. Patch 1 can be accepted without patch 2 and vice versa.
So we've decided to split the commits because patch 1 is in common
code, while patch 2 is in x86-specific code.
No other real reasons, but in any case we can move these changes to
patch 1.
>
>> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
>> @@ -1320,7 +1320,7 @@ x86_emulate(
>> ea.bytes = 2;
>> goto srcmem_common;
>> case SrcMem:
>> - if ( state->simd_size )
>> + if ( state->simd_size != simd_none )
>> break;
>> ea.bytes = (d & ByteOp) ? 1 : op_bytes;
>> srcmem_common:
>> @@ -1460,7 +1460,7 @@ x86_emulate(
>> /* Becomes a normal DstMem operation from here on. */
>> case DstMem:
>> generate_exception_if(ea.type == OP_MEM && evex.z, X86_EXC_UD);
>> - if ( state->simd_size )
>> + if ( state->simd_size != simd_none )
>> {
>> generate_exception_if(lock_prefix, X86_EXC_UD);
>> break;
>> @@ -8176,7 +8176,7 @@ x86_emulate(
>> goto done;
>> }
>>
>> - if ( state->rmw )
>> + if ( state->rmw != rmw_NONE )
>> {
>> ea.val = src.val;
>> op_bytes = dst.bytes;
>> @@ -8205,7 +8205,7 @@ x86_emulate(
>>
>> dst.type = OP_NONE;
>> }
>> - else if ( state->simd_size )
>> + else if ( state->simd_size != simd_none )
>> {
>> generate_exception_if(!op_bytes, X86_EXC_UD);
>> generate_exception_if((vex.opcx && (d & TwoOp) &&
>
> I'd be (somewhat reluctantly) okay with ack-ing this part of the patch.
>
> Jan
--
Simone Ballarin, M.Sc.
Field Application Engineer, BUGSENG (https://bugseng.com)
On 07.12.2023 14:53, Simone Ballarin wrote:
> On 07/12/23 11:54, Jan Beulich wrote:
>> On 07.12.2023 10:48, Simone Ballarin wrote:
>>> --- a/xen/arch/x86/hpet.c
>>> +++ b/xen/arch/x86/hpet.c
>>> @@ -279,7 +279,7 @@ static int hpet_msi_write(struct hpet_event_channel *ch, struct msi_msg *msg)
>>> {
>>> ch->msi.msg = *msg;
>>>
>>> - if ( iommu_intremap )
>>> + if ( iommu_intremap != iommu_intremap_off )
>>> {
>>> int rc = iommu_update_ire_from_msi(&ch->msi, msg);
>>>
>>> @@ -353,7 +353,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
>>> u32 cfg = hpet_read32(HPET_Tn_CFG(ch->idx));
>>> irq_desc_t *desc = irq_to_desc(ch->msi.irq);
>>>
>>> - if ( iommu_intremap )
>>> + if ( iommu_intremap != iommu_intremap_off )
>>> {
>>> ch->msi.hpet_id = hpet_blockid;
>>> ret = iommu_setup_hpet_msi(&ch->msi);
>>> @@ -372,7 +372,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
>>> ret = __hpet_setup_msi_irq(desc);
>>> if ( ret < 0 )
>>> {
>>> - if ( iommu_intremap )
>>> + if ( iommu_intremap != iommu_intremap_off )
>>> iommu_update_ire_from_msi(&ch->msi, NULL);
>>> return ret;
>>> }
>>> diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
>>> index 7f8e794254..72dce2e4ab 100644
>>> --- a/xen/arch/x86/msi.c
>>> +++ b/xen/arch/x86/msi.c
>>> @@ -189,7 +189,7 @@ static int write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
>>> {
>>> entry->msg = *msg;
>>>
>>> - if ( iommu_intremap )
>>> + if ( iommu_intremap != iommu_intremap_off )
>>> {
>>> int rc;
>>>
>>> @@ -555,7 +555,7 @@ int msi_free_irq(struct msi_desc *entry)
>>> destroy_irq(entry[nr].irq);
>>>
>>> /* Free the unused IRTE if intr remap enabled */
>>> - if ( iommu_intremap )
>>> + if ( iommu_intremap != iommu_intremap_off )
>>> iommu_update_ire_from_msi(entry + nr, NULL);
>>> }
>>>
>>
>> All of this would logically be part of patch 1. Is there a particular reason
>> why it wasn't done right there?
>
> These changes and the ones in patch 1 are related, but still remain
> independent. Patch 1 can be accepted without patch 2 and vice versa.
> So we've decided to split the commits because patch 1 is in common
> code, while patch 2 is in x86-specific code.
Just to clarify: While not located under arch/x86/, what patch 1 touches
is still x86-specific code. It's subject prefix also wrongly says
AMD/IOMMU: when it also touches VT-d code. Especially with the changes
here folded in, x86/IOMMU: might be more appropriate.
Jan
On 07/12/23 15:15, Jan Beulich wrote:
> On 07.12.2023 14:53, Simone Ballarin wrote:
>> On 07/12/23 11:54, Jan Beulich wrote:
>>> On 07.12.2023 10:48, Simone Ballarin wrote:
>>>> --- a/xen/arch/x86/hpet.c
>>>> +++ b/xen/arch/x86/hpet.c
>>>> @@ -279,7 +279,7 @@ static int hpet_msi_write(struct hpet_event_channel *ch, struct msi_msg *msg)
>>>> {
>>>> ch->msi.msg = *msg;
>>>>
>>>> - if ( iommu_intremap )
>>>> + if ( iommu_intremap != iommu_intremap_off )
>>>> {
>>>> int rc = iommu_update_ire_from_msi(&ch->msi, msg);
>>>>
>>>> @@ -353,7 +353,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
>>>> u32 cfg = hpet_read32(HPET_Tn_CFG(ch->idx));
>>>> irq_desc_t *desc = irq_to_desc(ch->msi.irq);
>>>>
>>>> - if ( iommu_intremap )
>>>> + if ( iommu_intremap != iommu_intremap_off )
>>>> {
>>>> ch->msi.hpet_id = hpet_blockid;
>>>> ret = iommu_setup_hpet_msi(&ch->msi);
>>>> @@ -372,7 +372,7 @@ static int __init hpet_setup_msi_irq(struct hpet_event_channel *ch)
>>>> ret = __hpet_setup_msi_irq(desc);
>>>> if ( ret < 0 )
>>>> {
>>>> - if ( iommu_intremap )
>>>> + if ( iommu_intremap != iommu_intremap_off )
>>>> iommu_update_ire_from_msi(&ch->msi, NULL);
>>>> return ret;
>>>> }
>>>> diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
>>>> index 7f8e794254..72dce2e4ab 100644
>>>> --- a/xen/arch/x86/msi.c
>>>> +++ b/xen/arch/x86/msi.c
>>>> @@ -189,7 +189,7 @@ static int write_msi_msg(struct msi_desc *entry, struct msi_msg *msg)
>>>> {
>>>> entry->msg = *msg;
>>>>
>>>> - if ( iommu_intremap )
>>>> + if ( iommu_intremap != iommu_intremap_off )
>>>> {
>>>> int rc;
>>>>
>>>> @@ -555,7 +555,7 @@ int msi_free_irq(struct msi_desc *entry)
>>>> destroy_irq(entry[nr].irq);
>>>>
>>>> /* Free the unused IRTE if intr remap enabled */
>>>> - if ( iommu_intremap )
>>>> + if ( iommu_intremap != iommu_intremap_off )
>>>> iommu_update_ire_from_msi(entry + nr, NULL);
>>>> }
>>>>
>>>
>>> All of this would logically be part of patch 1. Is there a particular reason
>>> why it wasn't done right there?
>>
>> These changes and the ones in patch 1 are related, but still remain
>> independent. Patch 1 can be accepted without patch 2 and vice versa.
>> So we've decided to split the commits because patch 1 is in common
>> code, while patch 2 is in x86-specific code.
>
> Just to clarify: While not located under arch/x86/, what patch 1 touches
> is still x86-specific code. It's subject prefix also wrongly says
> AMD/IOMMU: when it also touches VT-d code. Especially with the changes
> here folded in, x86/IOMMU: might be more appropriate.
>
OK, then I'll move the changes and use x86/IOMMU.
Thanks.
> Jan
>
--
Simone Ballarin, M.Sc.
Field Application Engineer, BUGSENG (https://bugseng.com)
© 2016 - 2026 Red Hat, Inc.