[RFC PATCH] hw/cxl: Align mailbox payload backing storage

[RFC PATCH] hw/cxl: Align mailbox payload backing storage

[Richard Cheng]

The Get Supported Features handler treats payload_out as a 16-byte
aligned structure. However, the mailbox register is aligned to 8 bytes.
Since the payload starts at A_CXL_DEV_CMD_PAYLOAD, a misaligned union
will produce a misaligned payload pointer.

The bug can be triggered when QEMU built with UBSAN:

  hw/cxl/cxl-mailbox-utils.c:1571:39: runtime error:
  member access within misaligned address 0xff7772f3d628 for type
  'struct <unknown>', which requires 16 byte alignment

QEMU aborts while handling GET_SUPPORTED_FEATURES

Didn't see a strong evidence why QEMU would treat it as 16-byte aligned,
so I want to raise the RFC and see what's the actual corrected fix,
rather than dropping the aligned tag directly.

Align the mailbox backing union to 16 bytes. Since the payload offset is
0x20, payload_out now satisfies the handlers' existing alignment
requirement.

Fixes: d80378943a6b ("hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6)")
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
Reproduce with:

    1. Configure QEMU with --enable-ubsan.
    2. Start QEMU with UBSAN_OPTIONS=halt_on_error=1 and a cxl-type3
       device.
    3. Boot a guest with CONFIG_CXL_FEATURES=y, or reprobe the endpoint:

       # echo <type3-bdf> > /sys/bus/pci/drivers/cxl_pci/unbind
       # echo <type3-bdf> > /sys/bus/pci/drivers/cxl_pci/bind

Best regards,
Richard Cheng.
---
 include/hw/cxl/cxl_device.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index ba551fa..2191b7c 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -248,7 +248,7 @@ typedef struct cxl_device_state {
             uint16_t mbox_reg_state16[CXL_MAILBOX_REGISTERS_LENGTH / 2];
             uint32_t mbox_reg_state32[CXL_MAILBOX_REGISTERS_LENGTH / 4];
             uint64_t mbox_reg_state64[CXL_MAILBOX_REGISTERS_LENGTH / 8];
-        };
+        } QEMU_ALIGNED(16);
     };
 
     /* Stash the memory device status value */
-- 
2.43.0

Re: [RFC PATCH] hw/cxl: Align mailbox payload backing storage

[Anisa Su]

On Tue, Oct 27, 2026 at 06:22:27PM +0800, Richard Cheng wrote:
> The Get Supported Features handler treats payload_out as a 16-byte
> aligned structure. However, the mailbox register is aligned to 8 bytes.
> Since the payload starts at A_CXL_DEV_CMD_PAYLOAD, a misaligned union
> will produce a misaligned payload pointer.
> 
> The bug can be triggered when QEMU built with UBSAN:
> 
>   hw/cxl/cxl-mailbox-utils.c:1571:39: runtime error:
>   member access within misaligned address 0xff7772f3d628 for type
>   'struct <unknown>', which requires 16 byte alignment
> 
> QEMU aborts while handling GET_SUPPORTED_FEATURES
> 
> Didn't see a strong evidence why QEMU would treat it as 16-byte aligned,
> so I want to raise the RFC and see what's the actual corrected fix,
> rather than dropping the aligned tag directly.
> 
> Align the mailbox backing union to 16 bytes. Since the payload offset is
> 0x20, payload_out now satisfies the handlers' existing alignment
> requirement.
> 
Hello Richard,

It looks like Jonathan already has this commit in his tree:

https://github.com/qualcomm/qemu/commit/271340b769bdf4b2614fcff76455a813f154d77b

I'm not really sure why the features handlers require QEMU_ALIGNED(16). I don't
see anything in the spec that requires the 16B alignment. And the Get Supported
Features input payload is padded from 8B to 16B because of QEMU_ALIGNED(16),
which seems not right?

The commit message from 271340b7 says:

"hw/cxl: Ensure the register space for the mailbox registers is 16 byte aligned.

Given there are some carefully aligned UUIDs in the registers,
enforce the alignment to solve ubscan issue..."

but if that refers to the UUID of output->feature_entries[x]:
the output payload has an 8B header, so the UUID of any feature entries
won't start at a 16B-aligned address anyway?

/*
 * Get Supported Features Supported Feature Entry
 * CXL r3.1 section 8.2.9.6.1 Table 8-97
 */
typedef struct CXLSupportedFeatureEntry {
    QemuUUID uuid;
    uint16_t feat_index;
    uint16_t get_feat_size;
    uint16_t set_feat_size;
    uint32_t attr_flags;
    uint8_t get_feat_version;
    uint8_t set_feat_version;
    uint16_t set_feat_effects;
    uint8_t rsvd[18];
} QEMU_PACKED CXLSupportedFeatureEntry;

struct {
        CXLSupportedFeatureHeader hdr; <--- 8B
        CXLSupportedFeatureEntry feat_entries[];
    }


If QEMU_ALIGNED(16) is not needed for Get Supported Features, then I think it should also
be dropped from the other Features handlers, and Get Log (Opcode 0401h).
I don't see anything in specification that requires 16B alignment for those
commands.

Thanks,
Anisa

> Fixes: d80378943a6b ("hw/cxl/cxl-mailbox-utils: Add support for feature commands (8.2.9.6)")
> Signed-off-by: Richard Cheng <icheng@nvidia.com>
> ---
> Reproduce with:
> 
>     1. Configure QEMU with --enable-ubsan.
>     2. Start QEMU with UBSAN_OPTIONS=halt_on_error=1 and a cxl-type3
>        device.
>     3. Boot a guest with CONFIG_CXL_FEATURES=y, or reprobe the endpoint:
> 
>        # echo <type3-bdf> > /sys/bus/pci/drivers/cxl_pci/unbind
>        # echo <type3-bdf> > /sys/bus/pci/drivers/cxl_pci/bind
> 
> Best regards,
> Richard Cheng.
> ---
>  include/hw/cxl/cxl_device.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index ba551fa..2191b7c 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -248,7 +248,7 @@ typedef struct cxl_device_state {
>              uint16_t mbox_reg_state16[CXL_MAILBOX_REGISTERS_LENGTH / 2];
>              uint32_t mbox_reg_state32[CXL_MAILBOX_REGISTERS_LENGTH / 4];
>              uint64_t mbox_reg_state64[CXL_MAILBOX_REGISTERS_LENGTH / 8];
> -        };
> +        } QEMU_ALIGNED(16);
>      };
>  
>      /* Stash the memory device status value */
> -- 
> 2.43.0
>