1. Patchew
  2. QEMU
  3. View series

[PATCH v2] hw/sensor: add stub for ADM1266 blackbox

Ben Hillier posted 1 patch 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250828184939.3140841-1-bhillier@google.com
Maintainers: Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
hw/sensor/adm1266.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
[PATCH v2] hw/sensor: add stub for ADM1266 blackbox
Posted by Ben Hillier 1 month ago 
From: Titus Rwantare <titusr@google.com>

This patch fixes a bug that causes a kernel panic when calling ADM1266_BLACKBOX_INFORMATION.

Log Message:

adm1266 5-0042: Could not read blackbox!
Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: adm1266_nvmem_read+0x168/0x170

Signed-off-by: Titus Rwantare <titusr@google.com>
---
 hw/sensor/adm1266.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/hw/sensor/adm1266.c b/hw/sensor/adm1266.c
index 9017ce6116..cd11526c2b 100644
--- a/hw/sensor/adm1266.c
+++ b/hw/sensor/adm1266.c
@@ -75,6 +75,7 @@ typedef struct ADM1266State {
 static const uint8_t adm1266_ic_device_id[] = {0x03, 0x41, 0x12, 0x66};
 static const uint8_t adm1266_ic_device_rev[] = {0x08, 0x01, 0x08, 0x07, 0x0,
                                                 0x0, 0x07, 0x41, 0x30};
+static const uint8_t adm1266_blackbox_info[] = {0x04, 0x00, 0x00, 0x00, 0x00};
 
 static void adm1266_exit_reset(Object *obj, ResetType type)
 {
@@ -125,6 +126,17 @@ static uint8_t adm1266_read_byte(PMBusDevice *pmdev)
         pmbus_send(pmdev, adm1266_ic_device_rev, sizeof(adm1266_ic_device_rev));
         break;
 
+    case ADM1266_BLACKBOX_CONFIG:
+        return 0;
+
+    case ADM1266_READ_BLACKBOX:
+        return 0;
+
+    case ADM1266_BLACKBOX_INFORMATION:
+        pmbus_send(pmdev, adm1266_blackbox_info,
+                   sizeof(adm1266_blackbox_info));
+        break;
+
     default:
         qemu_log_mask(LOG_UNIMP,
                       "%s: reading from unimplemented register: 0x%02x\n",
@@ -154,6 +166,9 @@ static int adm1266_write_data(PMBusDevice *pmdev, const uint8_t *buf,
         pmbus_receive_block(pmdev, (uint8_t *)s->mfr_rev, sizeof(s->mfr_rev));
         break;
 
+    case ADM1266_BLACKBOX_CONFIG:
+    case ADM1266_READ_BLACKBOX:
+    case ADM1266_BLACKBOX_INFORMATION:
     case ADM1266_SET_RTC:   /* do nothing */
         break;
 
-- 
2.51.0.rc0.155.g4a0f42376b-goog
Re: [PATCH v2] hw/sensor: add stub for ADM1266 blackbox
Posted by Philippe Mathieu-Daudé 4 weeks, 1 day ago 
Hi Titus,

On 28/8/25 20:49, Ben Hillier wrote:
> From: Titus Rwantare <titusr@google.com>
> 
> This patch fixes a bug that causes a kernel panic when calling ADM1266_BLACKBOX_INFORMATION.

But you also implement BLACKBOX_CONFIG / READ_BLACKBOX...

> Log Message:
> 
> adm1266 5-0042: Could not read blackbox!
> Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: adm1266_nvmem_read+0x168/0x170
> 
> Signed-off-by: Titus Rwantare <titusr@google.com>
> ---
>   hw/sensor/adm1266.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/hw/sensor/adm1266.c b/hw/sensor/adm1266.c
> index 9017ce6116..cd11526c2b 100644
> --- a/hw/sensor/adm1266.c
> +++ b/hw/sensor/adm1266.c
> @@ -75,6 +75,7 @@ typedef struct ADM1266State {
>   static const uint8_t adm1266_ic_device_id[] = {0x03, 0x41, 0x12, 0x66};
>   static const uint8_t adm1266_ic_device_rev[] = {0x08, 0x01, 0x08, 0x07, 0x0,
>                                                   0x0, 0x07, 0x41, 0x30};
> +static const uint8_t adm1266_blackbox_info[] = {0x04, 0x00, 0x00, 0x00, 0x00};
   {
     0x04,       /* length */
     0x00, 0x00, /* latest ID */
     0x00,       /* latest record logic index */
     0x00        /* latest record count */
   };

>   
>   static void adm1266_exit_reset(Object *obj, ResetType type)
>   {
> @@ -125,6 +126,17 @@ static uint8_t adm1266_read_byte(PMBusDevice *pmdev)
>           pmbus_send(pmdev, adm1266_ic_device_rev, sizeof(adm1266_ic_device_rev));
>           break;
>   
> +    case ADM1266_BLACKBOX_CONFIG:
> +        return 0;

I'd was expecting pmbus_send(pmdev, "\0\0", 2) for CYCLIC_RECORD_DISABLED.

> +
> +    case ADM1266_READ_BLACKBOX:

Here I'm confused, as len+data is 65bytes. Is the guest
reading a stream of 0xff?

> +        return 0;
> +
> +    case ADM1266_BLACKBOX_INFORMATION:
> +        pmbus_send(pmdev, adm1266_blackbox_info,
> +                   sizeof(adm1266_blackbox_info));
> +        break;

OK.

>       default:
>           qemu_log_mask(LOG_UNIMP,
>                         "%s: reading from unimplemented register: 0x%02x\n",
> @@ -154,6 +166,9 @@ static int adm1266_write_data(PMBusDevice *pmdev, const uint8_t *buf,
>           pmbus_receive_block(pmdev, (uint8_t *)s->mfr_rev, sizeof(s->mfr_rev));
>           break;
>   
> +    case ADM1266_BLACKBOX_CONFIG:
Do we want to check, if lduw_p(buf) != 0 -> log(UNIMP)?

> +    case ADM1266_READ_BLACKBOX:

Do we want to check, if (buf[0] != 1) -> log(GUEST_ERROR),
and log(UNIMP) regardless?

> +    case ADM1266_BLACKBOX_INFORMATION:

            log(GUEST_ERROR);

>       case ADM1266_SET_RTC:   /* do nothing */
>           break;
>   
Regards,

Phil.
Re: [PATCH v2] hw/sensor: add stub for ADM1266 blackbox
Posted by Titus Rwantare 2 weeks, 5 days ago 
On Fri, 29 Aug 2025 at 09:23, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi Titus,
>
> On 28/8/25 20:49, Ben Hillier wrote:
> > From: Titus Rwantare <titusr@google.com>
> >
> > This patch fixes a bug that causes a kernel panic when calling ADM1266_BLACKBOX_INFORMATION.
>
> But you also implement BLACKBOX_CONFIG / READ_BLACKBOX...
>
> > Log Message:
> >
> > adm1266 5-0042: Could not read blackbox!
> > Kernel panic - not syncing: stack-protector: Kernel stack is corrupted in: adm1266_nvmem_read+0x168/0x170
> >
> > Signed-off-by: Titus Rwantare <titusr@google.com>
> > ---
> >   hw/sensor/adm1266.c | 15 +++++++++++++++
> >   1 file changed, 15 insertions(+)
> >
> > diff --git a/hw/sensor/adm1266.c b/hw/sensor/adm1266.c
> > index 9017ce6116..cd11526c2b 100644
> > --- a/hw/sensor/adm1266.c
> > +++ b/hw/sensor/adm1266.c
> > @@ -75,6 +75,7 @@ typedef struct ADM1266State {
> >   static const uint8_t adm1266_ic_device_id[] = {0x03, 0x41, 0x12, 0x66};
> >   static const uint8_t adm1266_ic_device_rev[] = {0x08, 0x01, 0x08, 0x07, 0x0,
> >                                                   0x0, 0x07, 0x41, 0x30};
> > +static const uint8_t adm1266_blackbox_info[] = {0x04, 0x00, 0x00, 0x00, 0x00};
>    {
>      0x04,       /* length */
>      0x00, 0x00, /* latest ID */
>      0x00,       /* latest record logic index */
>      0x00        /* latest record count */
>    };
>
> >
> >   static void adm1266_exit_reset(Object *obj, ResetType type)
> >   {
> > @@ -125,6 +126,17 @@ static uint8_t adm1266_read_byte(PMBusDevice *pmdev)
> >           pmbus_send(pmdev, adm1266_ic_device_rev, sizeof(adm1266_ic_device_rev));
> >           break;
> >
> > +    case ADM1266_BLACKBOX_CONFIG:
> > +        return 0;
>
> I'd was expecting pmbus_send(pmdev, "\0\0", 2) for CYCLIC_RECORD_DISABLED.
>
> > +
> > +    case ADM1266_READ_BLACKBOX:
>
> Here I'm confused, as len+data is 65bytes. Is the guest
> reading a stream of 0xff?
>
> > +        return 0;
> > +
> > +    case ADM1266_BLACKBOX_INFORMATION:
> > +        pmbus_send(pmdev, adm1266_blackbox_info,
> > +                   sizeof(adm1266_blackbox_info));
> > +        break;
>
> OK.
>
> >       default:
> >           qemu_log_mask(LOG_UNIMP,
> >                         "%s: reading from unimplemented register: 0x%02x\n",
> > @@ -154,6 +166,9 @@ static int adm1266_write_data(PMBusDevice *pmdev, const uint8_t *buf,
> >           pmbus_receive_block(pmdev, (uint8_t *)s->mfr_rev, sizeof(s->mfr_rev));
> >           break;
> >
> > +    case ADM1266_BLACKBOX_CONFIG:
> Do we want to check, if lduw_p(buf) != 0 -> log(UNIMP)?
>
> > +    case ADM1266_READ_BLACKBOX:
>
> Do we want to check, if (buf[0] != 1) -> log(GUEST_ERROR),
> and log(UNIMP) regardless?
>
> > +    case ADM1266_BLACKBOX_INFORMATION:
>
>             log(GUEST_ERROR);
>
> >       case ADM1266_SET_RTC:   /* do nothing */
> >           break;
> >
> Regards,
>
> Phil.

Thanks for the feedback, I wound up implementing more of the blackbox
and implementing a test for it.
-Titus

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.