hw/intc/arm_gic.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
1. Possibly mismatched call arguments in function 'gic_apr_ns_view':
'cpu' and 'regno' passed in place of 'int regno' and 'int cpu'.
2. Possibly mismatched call arguments in function
'gic_apr_write_ns_view': 'cpu' and 'regno' passed in place of 'int
regno' and 'int cpu'.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
From 23b142f5046ba9d3aec57217f6d8f3127f9bff69 Mon Sep 17 00:00:00 2001
From: Andrey Shumilin <shum.sdl@nppct.ru>
Date: Sun, 5 May 2024 20:13:40 +0300
Subject: [PATCH] Patch hw/intc/arm_gic.c
Signed-off-by: Andrey Shumilin <shum.sdl@nppct.ru>
---
hw/intc/arm_gic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 7a34bc0998..47f01e45e3 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -1658,7 +1658,7 @@ static MemTxResult gic_cpu_read(GICState *s, int
cpu, int offset,
*data = s->h_apr[gic_get_vcpu_real_id(cpu)];
} else if (gic_cpu_ns_access(s, cpu, attrs)) {
/* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
- *data = gic_apr_ns_view(s, regno, cpu);
+ *data = gic_apr_ns_view(s, cpu, regno);
} else {
*data = s->apr[regno][cpu];
}
@@ -1746,7 +1746,7 @@ static MemTxResult gic_cpu_write(GICState *s, int
cpu, int offset,
s->h_apr[gic_get_vcpu_real_id(cpu)] = value;
} else if (gic_cpu_ns_access(s, cpu, attrs)) {
/* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
- gic_apr_write_ns_view(s, regno, cpu, value);
+ gic_apr_write_ns_view(s, cpu, regno, value);
} else {
s->apr[regno][cpu] = value;
}05.05.2024 20:58, Andrey Shumilin wrote: [unreadable text skipped] FWIW, your message is very difficult to read due to colors used within. /mjt
1. Possibly mismatched call arguments in function 'gic_apr_ns_view':
'cpu' and 'regno' passed in place of 'int regno' and 'int cpu'.
2. Possibly mismatched call arguments in function
'gic_apr_write_ns_view': 'cpu' and 'regno' passed in place of 'int
regno' and 'int cpu'.
Found by Linux Verification Center (linuxtesting.org) with SVACE.
From 23b142f5046ba9d3aec57217f6d8f3127f9bff69 Mon Sep 17 00:00:00 2001
From: Andrey Shumilin <shum.sdl@nppct.ru>
Date: Sun, 5 May 2024 20:13:40 +0300
Subject: [PATCH] Patch hw/intc/arm_gic.c
Signed-off-by: Andrey Shumilin <shum.sdl@nppct.ru>
---
hw/intc/arm_gic.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
index 7a34bc0998..47f01e45e3 100644
--- a/hw/intc/arm_gic.c
+++ b/hw/intc/arm_gic.c
@@ -1658,7 +1658,7 @@ static MemTxResult gic_cpu_read(GICState *s, int
cpu, int offset,
*data = s->h_apr[gic_get_vcpu_real_id(cpu)];
} else if (gic_cpu_ns_access(s, cpu, attrs)) {
/* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
- *data = gic_apr_ns_view(s, regno, cpu);
+ *data = gic_apr_ns_view(s, cpu, regno);
} else {
*data = s->apr[regno][cpu];
}
@@ -1746,7 +1746,7 @@ static MemTxResult gic_cpu_write(GICState *s, int
cpu, int offset,
s->h_apr[gic_get_vcpu_real_id(cpu)] = value;
} else if (gic_cpu_ns_access(s, cpu, attrs)) {
/* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
- gic_apr_write_ns_view(s, regno, cpu, value);
+ gic_apr_write_ns_view(s, cpu, regno, value);
} else {
s->apr[regno][cpu] = value;
}
On Sun, 5 May 2024 at 20:58, Andrey Shumilin <shum.sdl@nppct.ru> wrote: > > Possibly mismatched call arguments in function 'gic_apr_ns_view': 'cpu' and 'regno' passed in place of 'int regno' and 'int cpu'. > Possibly mismatched call arguments in function 'gic_apr_write_ns_view': 'cpu' and 'regno' passed in place of 'int regno' and 'int cpu'. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > > From 23b142f5046ba9d3aec57217f6d8f3127f9bff69 Mon Sep 17 00:00:00 2001 > From: Andrey Shumilin <shum.sdl@nppct.ru> > Date: Sun, 5 May 2024 20:13:40 +0300 > Subject: [PATCH] Patch hw/intc/arm_gic.c > > Signed-off-by: Andrey Shumilin <shum.sdl@nppct.ru> > --- > hw/intc/arm_gic.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > Thanks, I have applied your patch to target-arm.next (with a rewritten commit message). For future patches, it would be good if you can sort out the mail format issues. It looks like you're using Thunderbird, which ought to be configurable to do a reasonable job. In particular, if you can set it to send plain text only (not plain-text + HTML multipart) for all mailing list messages that will help a lot. That will help the automated tooling to have a better time trying to interpret it. You might alternatively consider git-send-email. thanks -- PMM
Andrey Shumilin <shum.sdl@nppct.ru> writes:
> 1 Possibly mismatched call arguments in function 'gic_apr_ns_view': 'cpu' and 'regno' passed in place of 'int regno' and 'int
> cpu'.
> 2 Possibly mismatched call arguments in function 'gic_apr_write_ns_view': 'cpu' and 'regno' passed in place of 'int regno' and
> 'int cpu'.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
So this purely a heuristic test based on the parameter names?
>
> From 23b142f5046ba9d3aec57217f6d8f3127f9bff69 Mon Sep 17 00:00:00 2001
> From: Andrey Shumilin <shum.sdl@nppct.ru>
> Date: Sun, 5 May 2024 20:13:40 +0300
> Subject: [PATCH] Patch hw/intc/arm_gic.c
>
> Signed-off-by: Andrey Shumilin <shum.sdl@nppct.ru>
> ---
> hw/intc/arm_gic.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 7a34bc0998..47f01e45e3 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -1658,7 +1658,7 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset,
> *data = s->h_apr[gic_get_vcpu_real_id(cpu)];
> } else if (gic_cpu_ns_access(s, cpu, attrs)) {
> /* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
> - *data = gic_apr_ns_view(s, regno, cpu);
> + *data = gic_apr_ns_view(s, cpu, regno);
> } else {
> *data = s->apr[regno][cpu];
> }
> @@ -1746,7 +1746,7 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
> s->h_apr[gic_get_vcpu_real_id(cpu)] = value;
> } else if (gic_cpu_ns_access(s, cpu, attrs)) {
> /* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
> - gic_apr_write_ns_view(s, regno, cpu, value);
> + gic_apr_write_ns_view(s, cpu, regno, value);
> } else {
> s->apr[regno][cpu] = value;
> }
Ahh C's lack of strong typing wins again :-/
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
1. s - This argument passes a pointer to the GICState structure that
contains the state of the interrupt controller. This argument is
passed first and used correctly.
2. regno - This is the register number, which in the context of
gic_cpu_read is calculated as (offset - 0xd0) / 4. In gic_cpu_read
code, this number is used to identify the specific APR register to
be read or modified. In gic_apr_ns_view, this number is also used to
determine which NSAPR register to read or how to compute bits,
implying that it is used as a register index.
3. cpu - This is the CPU number used to address a particular CPU in the
nsapr, apr, and other arrays.
Based on the context, it is important that regno and cpu are passed to
gic_apr_ns_view in the correct order for correct handling of arrays and
indexes within this function. An error in the order of the arguments can
result in incorrect data handling, as arrays are indexed first by CPU
number and then by register number. In the considered call
gic_apr_ns_view the arguments are passed in the wrong order
Fixes: 51fd06e0ee ("hw/intc/arm_gic: Fix handling of GICC_APR<n>,
GICC_NSAPR<n> registers")
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
06.05.2024 13:02, Alex Bennée пишет:
> Andrey Shumilin<shum.sdl@nppct.ru> writes:
>
>> 1 Possibly mismatched call arguments in function 'gic_apr_ns_view': 'cpu' and 'regno' passed in place of 'int regno' and 'int
>> cpu'.
>> 2 Possibly mismatched call arguments in function 'gic_apr_write_ns_view': 'cpu' and 'regno' passed in place of 'int regno' and
>> 'int cpu'.
>>
>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> So this purely a heuristic test based on the parameter names?
>
>> From 23b142f5046ba9d3aec57217f6d8f3127f9bff69 Mon Sep 17 00:00:00 2001
>> From: Andrey Shumilin<shum.sdl@nppct.ru>
>> Date: Sun, 5 May 2024 20:13:40 +0300
>> Subject: [PATCH] Patch hw/intc/arm_gic.c
>>
>> Signed-off-by: Andrey Shumilin<shum.sdl@nppct.ru>
>> ---
>> hw/intc/arm_gic.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
>> index 7a34bc0998..47f01e45e3 100644
>> --- a/hw/intc/arm_gic.c
>> +++ b/hw/intc/arm_gic.c
>> @@ -1658,7 +1658,7 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset,
>> *data = s->h_apr[gic_get_vcpu_real_id(cpu)];
>> } else if (gic_cpu_ns_access(s, cpu, attrs)) {
>> /* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
>> - *data = gic_apr_ns_view(s, regno, cpu);
>> + *data = gic_apr_ns_view(s, cpu, regno);
>> } else {
>> *data = s->apr[regno][cpu];
>> }
>> @@ -1746,7 +1746,7 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
>> s->h_apr[gic_get_vcpu_real_id(cpu)] = value;
>> } else if (gic_cpu_ns_access(s, cpu, attrs)) {
>> /* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
>> - gic_apr_write_ns_view(s, regno, cpu, value);
>> + gic_apr_write_ns_view(s, cpu, regno, value);
>> } else {
>> s->apr[regno][cpu] = value;
>> }
> Ahh C's lack of strong typing wins again :-/
>
> Reviewed-by: Alex Bennée<alex.bennee@linaro.org>
>
1. s - This argument passes a pointer to the GICState structure that
contains the state of the interrupt controller. This argument is
passed first and used correctly.
2. regno - This is the register number, which in the context of
gic_cpu_read is calculated as (offset - 0xd0) / 4. In gic_cpu_read
code, this number is used to identify the specific APR register to
be read or modified. In gic_apr_ns_view, this number is also used to
determine which NSAPR register to read or how to compute bits,
implying that it is used as a register index.
3. cpu - This is the CPU number used to address a particular CPU in the
nsapr, apr, and other arrays.
Based on the context, it is important that regno and cpu are passed to
gic_apr_ns_view in the correct order for correct handling of arrays and
indexes within this function. An error in the order of the arguments can
result in incorrect data handling, as arrays are indexed first by CPU
number and then by register number. In the considered call
gic_apr_ns_view the arguments are passed in the wrong order
06.05.2024 13:02, Alex Bennée пишет:
> Andrey Shumilin<shum.sdl@nppct.ru> writes:
>
>> 1 Possibly mismatched call arguments in function 'gic_apr_ns_view': 'cpu' and 'regno' passed in place of 'int regno' and 'int
>> cpu'.
>> 2 Possibly mismatched call arguments in function 'gic_apr_write_ns_view': 'cpu' and 'regno' passed in place of 'int regno' and
>> 'int cpu'.
>>
>> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> So this purely a heuristic test based on the parameter names?
>
>> From 23b142f5046ba9d3aec57217f6d8f3127f9bff69 Mon Sep 17 00:00:00 2001
>> From: Andrey Shumilin<shum.sdl@nppct.ru>
>> Date: Sun, 5 May 2024 20:13:40 +0300
>> Subject: [PATCH] Patch hw/intc/arm_gic.c
>>
>> Signed-off-by: Andrey Shumilin<shum.sdl@nppct.ru>
>> ---
>> hw/intc/arm_gic.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
>> index 7a34bc0998..47f01e45e3 100644
>> --- a/hw/intc/arm_gic.c
>> +++ b/hw/intc/arm_gic.c
>> @@ -1658,7 +1658,7 @@ static MemTxResult gic_cpu_read(GICState *s, int cpu, int offset,
>> *data = s->h_apr[gic_get_vcpu_real_id(cpu)];
>> } else if (gic_cpu_ns_access(s, cpu, attrs)) {
>> /* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
>> - *data = gic_apr_ns_view(s, regno, cpu);
>> + *data = gic_apr_ns_view(s, cpu, regno);
>> } else {
>> *data = s->apr[regno][cpu];
>> }
>> @@ -1746,7 +1746,7 @@ static MemTxResult gic_cpu_write(GICState *s, int cpu, int offset,
>> s->h_apr[gic_get_vcpu_real_id(cpu)] = value;
>> } else if (gic_cpu_ns_access(s, cpu, attrs)) {
>> /* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
>> - gic_apr_write_ns_view(s, regno, cpu, value);
>> + gic_apr_write_ns_view(s, cpu, regno, value);
>> } else {
>> s->apr[regno][cpu] = value;
>> }
> Ahh C's lack of strong typing wins again :-/
>
> Reviewed-by: Alex Bennée<alex.bennee@linaro.org>
>
On 5/5/24 21:57, Andrey Shumilin wrote:
> 1. Possibly mismatched call arguments in function 'gic_apr_ns_view':
> 'cpu' and 'regno' passed in place of 'int regno' and 'int cpu'.
> 2. Possibly mismatched call arguments in function
> 'gic_apr_write_ns_view': 'cpu' and 'regno' passed in place of 'int
> regno' and 'int cpu'.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
Fixes: 51fd06e0ee ("hw/intc/arm_gic: Fix handling of GICC_APR<n>,
GICC_NSAPR<n> registers")
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> From 23b142f5046ba9d3aec57217f6d8f3127f9bff69 Mon Sep 17 00:00:00 2001
> From: Andrey Shumilin <shum.sdl@nppct.ru>
> Date: Sun, 5 May 2024 20:13:40 +0300
> Subject: [PATCH] Patch hw/intc/arm_gic.c
Your patch is malformed, see:
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#submitting-your-patches
>
> Signed-off-by: Andrey Shumilin <shum.sdl@nppct.ru>
> ---
> hw/intc/arm_gic.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/arm_gic.c b/hw/intc/arm_gic.c
> index 7a34bc0998..47f01e45e3 100644
> --- a/hw/intc/arm_gic.c
> +++ b/hw/intc/arm_gic.c
> @@ -1658,7 +1658,7 @@ static MemTxResult gic_cpu_read(GICState *s, int
> cpu, int offset,
> *data = s->h_apr[gic_get_vcpu_real_id(cpu)];
> } else if (gic_cpu_ns_access(s, cpu, attrs)) {
> /* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
> - *data = gic_apr_ns_view(s, regno, cpu);
> + *data = gic_apr_ns_view(s, cpu, regno);
> } else {
> *data = s->apr[regno][cpu];
> }
> @@ -1746,7 +1746,7 @@ static MemTxResult gic_cpu_write(GICState *s, int
> cpu, int offset,
> s->h_apr[gic_get_vcpu_real_id(cpu)] = value;
> } else if (gic_cpu_ns_access(s, cpu, attrs)) {
> /* NS view of GICC_APR<n> is the top half of GIC_NSAPR<n> */
> - gic_apr_write_ns_view(s, regno, cpu, value);
> + gic_apr_write_ns_view(s, cpu, regno, value);
> } else {
> s->apr[regno][cpu] = value;
> }
>
© 2016 - 2024 Red Hat, Inc.