On Thu, Jul 23, 2026 at 11:18:46PM +0800, Bin Meng wrote:
> Polling firmware requests system services without requesting an
> interrupt. The old model ignored this distinction:
>
> 1. HSS requests the serial number with REQUEST set and NOTIFY clear.
> 2. QEMU synchronously fills the mailbox and clears REQUEST.
> 3. QEMU incorrectly asserts PLIC source 96.
> 4. HSS polls REQUEST and neither waits for nor handles the interrupt.
> 5. PLIC source 96 remains pending.
> 6. Linux registers the MPFS mailbox driver.
> 7. The stale interrupt immediately enters mpfs_mbox_inbox_isr().
> 8. Linux has not submitted a request or installed its response pointer.
> 9. The ISR dereferences that null pointer and faults near address 0x10.
>
> Honor SERVICES_CR.NOTIFY, let IOSCB own the pending interrupt state,
> and route the SYSREG acknowledgement through an IOSCB clear input.
>
> Signed-off-by: Bin Meng <[email protected]>
Reviewed-by: Chao Liu <[email protected]>
Thanks,
Chao
> ---
>
> include/hw/misc/mchp_pfsoc_ioscb.h | 2 ++
> hw/misc/mchp_pfsoc_ioscb.c | 39 +++++++++++++++++++++++++++---
> hw/misc/mchp_pfsoc_sysreg.c | 9 ++++++-
> hw/riscv/microchip_pfsoc.c | 6 ++---
> 4 files changed, 49 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/misc/mchp_pfsoc_ioscb.h
> b/include/hw/misc/mchp_pfsoc_ioscb.h
> index fd31427304..e39d995b64 100644
> --- a/include/hw/misc/mchp_pfsoc_ioscb.h
> +++ b/include/hw/misc/mchp_pfsoc_ioscb.h
> @@ -26,6 +26,7 @@
> #include "hw/core/sysbus.h"
>
> #define MCHP_PFSOC_IOSCB_MAILBOX_SIZE 0x1000
> +#define MCHP_PFSOC_IOSCB_IRQ_CLEAR "irq-clear"
>
> typedef struct MchpPfSoCIoscbState {
> SysBusDevice parent;
> @@ -53,6 +54,7 @@ typedef struct MchpPfSoCIoscbState {
> uint32_t services_sr;
> uint8_t mailbox_data[MCHP_PFSOC_IOSCB_MAILBOX_SIZE];
> char *serial_number;
> + bool irq_pending;
> qemu_irq irq;
> } MchpPfSoCIoscbState;
>
> diff --git a/hw/misc/mchp_pfsoc_ioscb.c b/hw/misc/mchp_pfsoc_ioscb.c
> index 7c82b55986..ccc2201b7a 100644
> --- a/hw/misc/mchp_pfsoc_ioscb.c
> +++ b/hw/misc/mchp_pfsoc_ioscb.c
> @@ -192,12 +192,13 @@ static const MemoryRegionOps
> mchp_pfsoc_io_calib_ddr_ops = {
>
> #define SERVICES_CR 0x50
> #define SERVICES_CR_REQUEST BIT(0)
> +#define SERVICES_CR_NOTIFY BIT(3)
> #define SERVICES_CR_COMMAND_SHIFT 16
> #define SERVICES_CR_COMMAND_WIDTH 8
> #define SERVICES_CR_COMMAND_MASK \
> MAKE_64BIT_MASK(SERVICES_CR_COMMAND_SHIFT, SERVICES_CR_COMMAND_WIDTH)
> #define SERVICES_CR_MASK \
> - (SERVICES_CR_REQUEST | SERVICES_CR_COMMAND_MASK)
> + (SERVICES_CR_REQUEST | SERVICES_CR_NOTIFY | SERVICES_CR_COMMAND_MASK)
> #define SERVICES_SR 0x54
> #define SERVICES_SR_STATUS_SHIFT 16
> #define SERVICES_COMMAND_SERIAL_NUMBER 0
> @@ -205,6 +206,21 @@ static const MemoryRegionOps mchp_pfsoc_io_calib_ddr_ops
> = {
> #define SERVICES_STATUS_FAILED 1
> #define SERVICES_MAILBOX_RESPONSE_OFFSET 0
>
> +static void mchp_pfsoc_ioscb_update_irq(MchpPfSoCIoscbState *s)
> +{
> + qemu_set_irq(s->irq, s->irq_pending);
> +}
> +
> +static void mchp_pfsoc_ioscb_clear_irq(void *opaque, int n, int level)
> +{
> + MchpPfSoCIoscbState *s = opaque;
> +
> + if (level) {
> + s->irq_pending = false;
> + mchp_pfsoc_ioscb_update_irq(s);
> + }
> +}
> +
> static void services_cr_write(MchpPfSoCIoscbState *s, uint32_t value)
> {
> uint32_t command;
> @@ -236,7 +252,15 @@ static void services_cr_write(MchpPfSoCIoscbState *s,
> uint32_t value)
> }
>
> s->services_sr = status << SERVICES_SR_STATUS_SHIFT;
> - qemu_irq_raise(s->irq);
> + /*
> + * HSS and U-Boot submit polling requests with REQUEST set and NOTIFY
> + * clear, then poll REQUEST/BUSY for completion. Linux sets both bits
> + * and expects completion through PLIC source 96.
> + */
> + if (value & SERVICES_CR_NOTIFY) {
> + s->irq_pending = true;
> + mchp_pfsoc_ioscb_update_irq(s);
> + }
> }
>
> static uint64_t mchp_pfsoc_ctrl_read(void *opaque, hwaddr offset,
> @@ -325,7 +349,8 @@ static void mchp_pfsoc_ioscb_reset(DeviceState *dev)
> s->services_cr = 0;
> s->services_sr = 0;
> memset(s->mailbox_data, 0, sizeof(s->mailbox_data));
> - qemu_irq_lower(s->irq);
> + s->irq_pending = false;
> + mchp_pfsoc_ioscb_update_irq(s);
> }
>
> static const Property mchp_pfsoc_ioscb_properties[] = {
> @@ -333,6 +358,13 @@ static const Property mchp_pfsoc_ioscb_properties[] = {
> MchpPfSoCIoscbState, serial_number),
> };
>
> +static void mchp_pfsoc_ioscb_init(Object *obj)
> +{
> + /* Accept service interrupt acknowledgements from SYSREG MESSAGE_INT */
> + qdev_init_gpio_in_named(DEVICE(obj), mchp_pfsoc_ioscb_clear_irq,
> + MCHP_PFSOC_IOSCB_IRQ_CLEAR, 1);
> +}
> +
> static void mchp_pfsoc_ioscb_realize(DeviceState *dev, Error **errp)
> {
> MchpPfSoCIoscbState *s = MCHP_PFSOC_IOSCB(dev);
> @@ -456,6 +488,7 @@ static const TypeInfo mchp_pfsoc_ioscb_info = {
> .name = TYPE_MCHP_PFSOC_IOSCB,
> .parent = TYPE_SYS_BUS_DEVICE,
> .instance_size = sizeof(MchpPfSoCIoscbState),
> + .instance_init = mchp_pfsoc_ioscb_init,
> .class_init = mchp_pfsoc_ioscb_class_init,
> };
>
> diff --git a/hw/misc/mchp_pfsoc_sysreg.c b/hw/misc/mchp_pfsoc_sysreg.c
> index 1d9154280a..899b485da6 100644
> --- a/hw/misc/mchp_pfsoc_sysreg.c
> +++ b/hw/misc/mchp_pfsoc_sysreg.c
> @@ -77,7 +77,14 @@ static void mchp_pfsoc_sysreg_write(void *opaque, hwaddr
> offset,
> }
> break;
> case MESSAGE_INT:
> - qemu_irq_lower(s->irq);
> + /*
> + * A MESSAGE_INT write is an acknowledgement event, not a level that
> + * remains asserted. Model it as an active-high pulse. The rising
> edge
> + * invokes IOSCB's irq-clear input with level 1, which clears
> + * irq_pending and lowers PLIC source 96. The falling edge invokes
> the
> + * input with level 0 and is ignored.
> + */
> + qemu_irq_pulse(s->irq);
> break;
> default:
> qemu_log_mask(LOG_UNIMP, "%s: unimplemented device write "
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index 63658fbeb0..f348de6fd4 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -333,9 +333,6 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev,
> Error **errp)
> sysbus_realize(SYS_BUS_DEVICE(&s->sysreg), errp);
> sysbus_mmio_map(SYS_BUS_DEVICE(&s->sysreg), 0,
> memmap[MICROCHIP_PFSOC_SYSREG].base);
> - sysbus_connect_irq(SYS_BUS_DEVICE(&s->sysreg), 0,
> - qdev_get_gpio_in(DEVICE(s->plic),
> - MICROCHIP_PFSOC_MAILBOX_IRQ));
>
> /* AXISW */
> create_unimplemented_device("microchip.pfsoc.axisw",
> @@ -489,6 +486,9 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev,
> Error **errp)
> sysbus_connect_irq(SYS_BUS_DEVICE(&s->ioscb), 0,
> qdev_get_gpio_in(DEVICE(s->plic),
> MICROCHIP_PFSOC_MAILBOX_IRQ));
> + sysbus_connect_irq(SYS_BUS_DEVICE(&s->sysreg), 0,
> + qdev_get_gpio_in_named(DEVICE(&s->ioscb),
> + MCHP_PFSOC_IOSCB_IRQ_CLEAR, 0));
>
> /* FPGA Fabric */
> create_unimplemented_device("microchip.pfsoc.fabricfic3",
> --
> 2.34.1
>
