1. Patchew
  2. QEMU
  3. View series

[RFC PATCH] hw/i2c/aspeed_i2c: Make AspeedI2CClass::gap an plain unsigned type

Philippe Mathieu-Daudé posted 1 patch 6 months, 3 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250423120555.21318-1-philmd@linaro.org
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>
include/hw/i2c/aspeed_i2c.h | 2 +-
hw/i2c/aspeed_i2c.c         | 5 +----
2 files changed, 2 insertions(+), 5 deletions(-)
[RFC PATCH] hw/i2c/aspeed_i2c: Make AspeedI2CClass::gap an plain unsigned type
Posted by Philippe Mathieu-Daudé 6 months, 3 weeks ago 
Convert AspeedI2CClass::gap to plain unsigned, using '0'
as "no gap" to avoid the followin UBSan warnings:

  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1559:16
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1583:16
  SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1608:16
  hw/i2c/aspeed_i2c.c:1608:16: runtime error: implicit conversion from type 'int' of value
                               -1 (32-bit, signed) to type 'uint8_t' (aka 'unsigned char')
                               changed the value to 255 (8-bit, unsigned)

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/i2c/aspeed_i2c.h | 2 +-
 hw/i2c/aspeed_i2c.c         | 5 +----
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
index 2c4c81bd209..098356e5bac 100644
--- a/include/hw/i2c/aspeed_i2c.h
+++ b/include/hw/i2c/aspeed_i2c.h
@@ -290,7 +290,7 @@ struct AspeedI2CClass {
     uint8_t num_busses;
     uint8_t reg_size;
     uint32_t reg_gap_size;
-    uint8_t gap;
+    unsigned gap;
     qemu_irq (*bus_get_irq)(AspeedI2CBus *);
 
     uint64_t pool_size;
diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
index a8fbb9f44a1..a45a4fd6cb7 100644
--- a/hw/i2c/aspeed_i2c.c
+++ b/hw/i2c/aspeed_i2c.c
@@ -1215,7 +1215,7 @@ static void aspeed_i2c_realize(DeviceState *dev, Error **errp)
 
     for (i = 0; i < aic->num_busses; i++) {
         Object *bus = OBJECT(&s->busses[i]);
-        int offset = i < aic->gap ? 1 : 5;
+        unsigned offset = i < aic->gap ? 1 : 5;
 
         if (!object_property_set_link(bus, "controller", OBJECT(s), errp)) {
             return;
@@ -1556,7 +1556,6 @@ static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data)
 
     aic->num_busses = 16;
     aic->reg_size = 0x80;
-    aic->gap = -1; /* no gap */
     aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq;
     aic->pool_size = 0x20;
     aic->pool_base = 0xC00;
@@ -1580,7 +1579,6 @@ static void aspeed_1030_i2c_class_init(ObjectClass *klass, void *data)
 
     aic->num_busses = 14;
     aic->reg_size = 0x80;
-    aic->gap = -1; /* no gap */
     aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq;
     aic->pool_size = 0x20;
     aic->pool_base = 0xC00;
@@ -1605,7 +1603,6 @@ static void aspeed_2700_i2c_class_init(ObjectClass *klass, void *data)
     aic->num_busses = 16;
     aic->reg_size = 0x80;
     aic->reg_gap_size = 0x80;
-    aic->gap = -1; /* no gap */
     aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq;
     aic->pool_size = 0x20;
     aic->pool_gap_size = 0xe0;
-- 
2.47.1
Re: [RFC PATCH] hw/i2c/aspeed_i2c: Make AspeedI2CClass::gap an plain unsigned type
Posted by Cédric Le Goater 6 months, 3 weeks ago 
On 4/23/25 14:05, Philippe Mathieu-Daudé wrote:
> Convert AspeedI2CClass::gap to plain unsigned, using '0'
> as "no gap" to avoid the followin UBSan warnings:
> 
>    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1559:16
>    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1583:16
>    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1608:16
>    hw/i2c/aspeed_i2c.c:1608:16: runtime error: implicit conversion from type 'int' of value
>                                 -1 (32-bit, signed) to type 'uint8_t' (aka 'unsigned char')
>                                 changed the value to 255 (8-bit, unsigned)
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Looks fine.


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/hw/i2c/aspeed_i2c.h | 2 +-
>   hw/i2c/aspeed_i2c.c         | 5 +----
>   2 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/include/hw/i2c/aspeed_i2c.h b/include/hw/i2c/aspeed_i2c.h
> index 2c4c81bd209..098356e5bac 100644
> --- a/include/hw/i2c/aspeed_i2c.h
> +++ b/include/hw/i2c/aspeed_i2c.h
> @@ -290,7 +290,7 @@ struct AspeedI2CClass {
>       uint8_t num_busses;
>       uint8_t reg_size;
>       uint32_t reg_gap_size;
> -    uint8_t gap;
> +    unsigned gap;
>       qemu_irq (*bus_get_irq)(AspeedI2CBus *);
>   
>       uint64_t pool_size;
> diff --git a/hw/i2c/aspeed_i2c.c b/hw/i2c/aspeed_i2c.c
> index a8fbb9f44a1..a45a4fd6cb7 100644
> --- a/hw/i2c/aspeed_i2c.c
> +++ b/hw/i2c/aspeed_i2c.c
> @@ -1215,7 +1215,7 @@ static void aspeed_i2c_realize(DeviceState *dev, Error **errp)
>   
>       for (i = 0; i < aic->num_busses; i++) {
>           Object *bus = OBJECT(&s->busses[i]);
> -        int offset = i < aic->gap ? 1 : 5;
> +        unsigned offset = i < aic->gap ? 1 : 5;
>   
>           if (!object_property_set_link(bus, "controller", OBJECT(s), errp)) {
>               return;
> @@ -1556,7 +1556,6 @@ static void aspeed_2600_i2c_class_init(ObjectClass *klass, void *data)
>   
>       aic->num_busses = 16;
>       aic->reg_size = 0x80;
> -    aic->gap = -1; /* no gap */
>       aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq;
>       aic->pool_size = 0x20;
>       aic->pool_base = 0xC00;
> @@ -1580,7 +1579,6 @@ static void aspeed_1030_i2c_class_init(ObjectClass *klass, void *data)
>   
>       aic->num_busses = 14;
>       aic->reg_size = 0x80;
> -    aic->gap = -1; /* no gap */
>       aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq;
>       aic->pool_size = 0x20;
>       aic->pool_base = 0xC00;
> @@ -1605,7 +1603,6 @@ static void aspeed_2700_i2c_class_init(ObjectClass *klass, void *data)
>       aic->num_busses = 16;
>       aic->reg_size = 0x80;
>       aic->reg_gap_size = 0x80;
> -    aic->gap = -1; /* no gap */
>       aic->bus_get_irq = aspeed_2600_i2c_bus_get_irq;
>       aic->pool_size = 0x20;
>       aic->pool_gap_size = 0xe0;
Re: [RFC PATCH] hw/i2c/aspeed_i2c: Make AspeedI2CClass::gap an plain unsigned type
Posted by Cédric Le Goater 6 months, 3 weeks ago 
On 4/23/25 15:33, Cédric Le Goater wrote:
> On 4/23/25 14:05, Philippe Mathieu-Daudé wrote:
>> Convert AspeedI2CClass::gap to plain unsigned, using '0'
>> as "no gap" to avoid the followin UBSan warnings:
>>
>>    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1559:16
>>    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1583:16
>>    SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../../hw/i2c/aspeed_i2c.c:1608:16
>>    hw/i2c/aspeed_i2c.c:1608:16: runtime error: implicit conversion from type 'int' of value
>>                                 -1 (32-bit, signed) to type 'uint8_t' (aka 'unsigned char')
>>                                 changed the value to 255 (8-bit, unsigned)
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
> Looks fine.

nope. It's breaking make check :


  ERROR:../tests/qtest/tpm-tis-i2c-test.c:104:tpm_tis_i2c_test_basic: assertion failed (access == TPM_TIS_ACCESS_TPM_REG_VALID_STS | TPM_TIS_ACCESS_TPM_ESTABLISHMENT): (255 == 129)
Unexpected error in qio_channel_socket_writev() at ../io/channel-socket.c:622:
/home/legoater/work/qemu/qemu-aspeed.git/build/tests/qtest/tpm-tis-i2c-test: Unable to write to socket: Bad file descriptorqemu-system-aarch64:
  tpm-emulator: Could not cleanly shutdown the TPM: Interrupted system call


C.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.