Re: [RFC PATCH v3 05/31] hw/cxl/device: Implement basic mailbox (8.2.8.4)
On Wed, 17 Feb 2021 16:55:14 -0800
Ben Widawsky wrote:
> On 21-02-11 17:46:39, Jonathan Cameron wrote:
> > On Tue, 2 Feb 2021 14:58:30 +
> > Jonathan Cameron wrote:
> >
> > > On Mon, 1 Feb 2021 16:59:22 -0800
> > > Ben Widawsky wrote:
> > >
> > > > This is the beginning of implementing mailbox support for CXL 2.0
> > > > devices. The implementation recognizes when the doorbell is rung,
> > > > handles the command/payload, clears the doorbell while returning error
> > > > codes and data.
> > > >
> > > > Generally the mailbox mechanism is designed to permit communication
> > > > between the host OS and the firmware running on the device. For our
> > > > purposes, we emulate both the firmware, implemented primarily in
> > > > cxl-mailbox-utils.c, and the hardware.
> > > >
> > > > No commands are implemented yet.
> > > >
> > > > Signed-off-by: Ben Widawsky
> > > > ---
> > > > hw/cxl/cxl-device-utils.c | 125 ++-
> > > > hw/cxl/cxl-mailbox-utils.c | 197
> > > > hw/cxl/meson.build | 1 +
> > > > include/hw/cxl/cxl.h| 3 +
> > > > include/hw/cxl/cxl_device.h | 28 -
> > > > 5 files changed, 349 insertions(+), 5 deletions(-)
> > > > create mode 100644 hw/cxl/cxl-mailbox-utils.c
> > > >
> > > > diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> > > > index bb15ad9a0f..6602606f3d 100644
> > > > --- a/hw/cxl/cxl-device-utils.c
> > > > +++ b/hw/cxl/cxl-device-utils.c
> > > > @@ -40,6 +40,111 @@ static uint64_t dev_reg_read(void *opaque, hwaddr
> > > > offset, unsigned size)
> > > > return 0;
> > > > }
> > > >
> > > > +static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned
> > > > size)
> > > > +{
> > > > +CXLDeviceState *cxl_dstate = opaque;
> > > > +
> > > > +switch (size) {
> >
> > As per the thread on the linux driver and the infinite loop I saw there
> > as a result of doing 1 byte reads.
> >
> > With the current setup of min_access_size = 4 and this
> > function QEMU will helpfully issue a series of unaligned 4 byte
> > reads to this function. It will then mask those down to 1 byte each
> > and combine them. Given the integer division that results in
> > the bottom byte of offset / 4 being returned up to 4 times.
> >
> > To handle 2 and 1 byte reads we need explicit support in here and
> > the MemoryRegionOps need to reflect that as well.
> >
> > All the similar cases where such reads are allowed need to do the
> > same.
> >
>
> From the documentation (memory.rst)
> - .impl.min_access_size, .impl.max_access_size define the access sizes
> (in bytes) supported by the *implementation*; other access sizes will be
> emulated using the ones available. For example a 4-byte write will be
> emulated using four 1-byte writes, if .impl.max_access_size = 1.
>
> I'm intentionally not looking at the implementation. As I read this, the
> behavior you describe is either a QEMU bug, or poor documentation.
>
> Am I missing something?
Good question...
For the case of emulating a larger access than supported it seems to be
fine
Code is implemented in softmmu/memory.c as something like.
for (i = 0; i < size; i+= access_size) {
r |= access_fn(mr, addr + i, value, access_size,
i * 8, access_mask, attrs);
}
For smaller read than supported,
memory_region_shift_read_access() looks like it would do the
masking but the mask and shift passed to the function are wrong.
There is a fixme in the code referring to unaligned accesses,
but these are aligned.
It's fairly easy to make this work, but unexpected side effects
worry me a little + my stab at big endian support may be completely
wrong.
I'll mess around with this a bit more then send out something to
see how people would like to fix this issue
(either the docs, or the code).
Jonathan
>
> > > > +case 8:
> > > > +return cxl_dstate->mbox_reg_state64[offset / 8];
> > > > +case 4:
> > > > +return cxl_dstate->mbox_reg_state32[offset / 4];
> > >
> > > Numeric order seems more natural and I can't see a reason not to do it.
> > > + you do them in that order below.
> > >
> > > > +default:
> > > > +g_assert_not_reached();
> > > > +}
> > > > +}
> > > > +
> > > > +static void mailbox_mem_writel(uint32_t *reg_state, hwaddr offset,
> > > > + uint64_t value)
> > > > +{
> > > > +switch (offset) {
> > > > +case A_CXL_DEV_MAILBOX_CTRL:
> > > > +/* fallthrough */
> > > > +case A_CXL_DEV_MAILBOX_CAP:
> > > > +/* RO register */
> > > > +break;
> > > > +default:
> > > > +qemu_log_mask(LOG_UNIMP,
> > > > + "%s Unexpected 32-bit access to 0x%" PRIx64 "
> > > > (WI)\n",
> > > > + __func__, offset);
> > > > +break;
> > > > +}
> > > > +
> > > > +reg_state[offset / 4] = value;
> > > > +}
> > > > +
> > > > +static void
Re: [RFC PATCH v3 05/31] hw/cxl/device: Implement basic mailbox (8.2.8.4)
On 21-02-11 17:46:39, Jonathan Cameron wrote:
> On Tue, 2 Feb 2021 14:58:30 +
> Jonathan Cameron wrote:
>
> > On Mon, 1 Feb 2021 16:59:22 -0800
> > Ben Widawsky wrote:
> >
> > > This is the beginning of implementing mailbox support for CXL 2.0
> > > devices. The implementation recognizes when the doorbell is rung,
> > > handles the command/payload, clears the doorbell while returning error
> > > codes and data.
> > >
> > > Generally the mailbox mechanism is designed to permit communication
> > > between the host OS and the firmware running on the device. For our
> > > purposes, we emulate both the firmware, implemented primarily in
> > > cxl-mailbox-utils.c, and the hardware.
> > >
> > > No commands are implemented yet.
> > >
> > > Signed-off-by: Ben Widawsky
> > > ---
> > > hw/cxl/cxl-device-utils.c | 125 ++-
> > > hw/cxl/cxl-mailbox-utils.c | 197
> > > hw/cxl/meson.build | 1 +
> > > include/hw/cxl/cxl.h| 3 +
> > > include/hw/cxl/cxl_device.h | 28 -
> > > 5 files changed, 349 insertions(+), 5 deletions(-)
> > > create mode 100644 hw/cxl/cxl-mailbox-utils.c
> > >
> > > diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> > > index bb15ad9a0f..6602606f3d 100644
> > > --- a/hw/cxl/cxl-device-utils.c
> > > +++ b/hw/cxl/cxl-device-utils.c
> > > @@ -40,6 +40,111 @@ static uint64_t dev_reg_read(void *opaque, hwaddr
> > > offset, unsigned size)
> > > return 0;
> > > }
> > >
> > > +static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned
> > > size)
> > > +{
> > > +CXLDeviceState *cxl_dstate = opaque;
> > > +
> > > +switch (size) {
>
> As per the thread on the linux driver and the infinite loop I saw there
> as a result of doing 1 byte reads.
>
> With the current setup of min_access_size = 4 and this
> function QEMU will helpfully issue a series of unaligned 4 byte
> reads to this function. It will then mask those down to 1 byte each
> and combine them. Given the integer division that results in
> the bottom byte of offset / 4 being returned up to 4 times.
>
> To handle 2 and 1 byte reads we need explicit support in here and
> the MemoryRegionOps need to reflect that as well.
>
> All the similar cases where such reads are allowed need to do the
> same.
>
>From the documentation (memory.rst)
- .impl.min_access_size, .impl.max_access_size define the access sizes
(in bytes) supported by the *implementation*; other access sizes will be
emulated using the ones available. For example a 4-byte write will be
emulated using four 1-byte writes, if .impl.max_access_size = 1.
I'm intentionally not looking at the implementation. As I read this, the
behavior you describe is either a QEMU bug, or poor documentation.
Am I missing something?
> > > +case 8:
> > > +return cxl_dstate->mbox_reg_state64[offset / 8];
> > > +case 4:
> > > +return cxl_dstate->mbox_reg_state32[offset / 4];
> >
> > Numeric order seems more natural and I can't see a reason not to do it.
> > + you do them in that order below.
> >
> > > +default:
> > > +g_assert_not_reached();
> > > +}
> > > +}
> > > +
> > > +static void mailbox_mem_writel(uint32_t *reg_state, hwaddr offset,
> > > + uint64_t value)
> > > +{
> > > +switch (offset) {
> > > +case A_CXL_DEV_MAILBOX_CTRL:
> > > +/* fallthrough */
> > > +case A_CXL_DEV_MAILBOX_CAP:
> > > +/* RO register */
> > > +break;
> > > +default:
> > > +qemu_log_mask(LOG_UNIMP,
> > > + "%s Unexpected 32-bit access to 0x%" PRIx64 "
> > > (WI)\n",
> > > + __func__, offset);
> > > +break;
> > > +}
> > > +
> > > +reg_state[offset / 4] = value;
> > > +}
> > > +
> > > +static void mailbox_mem_writeq(uint64_t *reg_state, hwaddr offset,
> > > + uint64_t value)
> > > +{
> > > +switch (offset) {
> > > +case A_CXL_DEV_MAILBOX_CMD:
> > > +break;
> > > +case A_CXL_DEV_BG_CMD_STS:
> > > +/* BG not supported */
> > > +/* fallthrough */
> > > +case A_CXL_DEV_MAILBOX_STS:
> > > +/* Read only register, will get updated by the state machine */
> > > +return;
> > > +default:
> > > +qemu_log_mask(LOG_UNIMP,
> > > + "%s Unexpected 64-bit access to 0x%" PRIx64 "
> > > (WI)\n",
> > > + __func__, offset);
> > > +return;
> > > +}
> > > +
> > > +
> > > +reg_state[offset / 8] = value;
> > > +}
> > > +
> > > +static void mailbox_reg_write(void *opaque, hwaddr offset, uint64_t
> > > value,
> > > + unsigned size)
> > > +{
> > > +CXLDeviceState *cxl_dstate = opaque;
> > > +
> > > +if (offset >= A_CXL_DEV_CMD_PAYLOAD) {
> > > +memcpy(cxl_dstate->mbox_reg_state + offset, , size);
> > > +return;
> > > +}
Re: [RFC PATCH v3 05/31] hw/cxl/device: Implement basic mailbox (8.2.8.4)
On Mon, 1 Feb 2021 16:59:22 -0800
Ben Widawsky wrote:
> This is the beginning of implementing mailbox support for CXL 2.0
> devices. The implementation recognizes when the doorbell is rung,
> handles the command/payload, clears the doorbell while returning error
> codes and data.
>
> Generally the mailbox mechanism is designed to permit communication
> between the host OS and the firmware running on the device. For our
> purposes, we emulate both the firmware, implemented primarily in
> cxl-mailbox-utils.c, and the hardware.
>
> No commands are implemented yet.
>
> Signed-off-by: Ben Widawsky
Sorry review is a little incoherent. It's a lot of patches
so I've ended up looking at your tree then trying to figure out
which patch a given comment belongs alongside.
> ---
> hw/cxl/cxl-device-utils.c | 125 ++-
> hw/cxl/cxl-mailbox-utils.c | 197
> hw/cxl/meson.build | 1 +
> include/hw/cxl/cxl.h| 3 +
> include/hw/cxl/cxl_device.h | 28 -
> 5 files changed, 349 insertions(+), 5 deletions(-)
> create mode 100644 hw/cxl/cxl-mailbox-utils.c
>
> diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> index bb15ad9a0f..6602606f3d 100644
> --- a/hw/cxl/cxl-device-utils.c
> +++ b/hw/cxl/cxl-device-utils.c
> @@ -40,6 +40,111 @@ static uint64_t dev_reg_read(void *opaque, hwaddr offset,
> unsigned size)
> return 0;
> }
>
> +
> +#define define_mailbox_handler_zeroed(name, size) \
> +uint16_t __zero##name = size; \
> +static ret_code cmd_##name(struct cxl_cmd *cmd, \
> + CXLDeviceState *cxl_dstate, uint16_t *len) \
> +{ \
> +*len = __zero##name; \
Why not just use the value of size here?
__zero##name isn't used anywhere else.
> +memset(cmd->payload, 0, *len);\
> +return CXL_MBOX_SUCCESS; \
> +}
> +#define define_mailbox_handler_const(name, data) \
> +static ret_code cmd_##name(struct cxl_cmd *cmd, \
> + CXLDeviceState *cxl_dstate, uint16_t *len) \
> +{ \
> +*len = sizeof(data); \
> +memcpy(cmd->payload, data, *len); \
> +return CXL_MBOX_SUCCESS; \
> +}
> +#define define_mailbox_handler_nop(name) \
> +static ret_code cmd_##name(struct cxl_cmd *cmd, \
> + CXLDeviceState *cxl_dstate, uint16_t *len) \
> +{ \
> +return CXL_MBOX_SUCCESS; \
> +}
> +
Re: [RFC PATCH v3 05/31] hw/cxl/device: Implement basic mailbox (8.2.8.4)
On Tue, 2 Feb 2021 14:58:30 +
Jonathan Cameron wrote:
> On Mon, 1 Feb 2021 16:59:22 -0800
> Ben Widawsky wrote:
>
> > This is the beginning of implementing mailbox support for CXL 2.0
> > devices. The implementation recognizes when the doorbell is rung,
> > handles the command/payload, clears the doorbell while returning error
> > codes and data.
> >
> > Generally the mailbox mechanism is designed to permit communication
> > between the host OS and the firmware running on the device. For our
> > purposes, we emulate both the firmware, implemented primarily in
> > cxl-mailbox-utils.c, and the hardware.
> >
> > No commands are implemented yet.
> >
> > Signed-off-by: Ben Widawsky
> > ---
> > hw/cxl/cxl-device-utils.c | 125 ++-
> > hw/cxl/cxl-mailbox-utils.c | 197
> > hw/cxl/meson.build | 1 +
> > include/hw/cxl/cxl.h| 3 +
> > include/hw/cxl/cxl_device.h | 28 -
> > 5 files changed, 349 insertions(+), 5 deletions(-)
> > create mode 100644 hw/cxl/cxl-mailbox-utils.c
> >
> > diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> > index bb15ad9a0f..6602606f3d 100644
> > --- a/hw/cxl/cxl-device-utils.c
> > +++ b/hw/cxl/cxl-device-utils.c
> > @@ -40,6 +40,111 @@ static uint64_t dev_reg_read(void *opaque, hwaddr
> > offset, unsigned size)
> > return 0;
> > }
> >
> > +static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned
> > size)
> > +{
> > +CXLDeviceState *cxl_dstate = opaque;
> > +
> > +switch (size) {
As per the thread on the linux driver and the infinite loop I saw there
as a result of doing 1 byte reads.
With the current setup of min_access_size = 4 and this
function QEMU will helpfully issue a series of unaligned 4 byte
reads to this function. It will then mask those down to 1 byte each
and combine them. Given the integer division that results in
the bottom byte of offset / 4 being returned up to 4 times.
To handle 2 and 1 byte reads we need explicit support in here and
the MemoryRegionOps need to reflect that as well.
All the similar cases where such reads are allowed need to do the
same.
> > +case 8:
> > +return cxl_dstate->mbox_reg_state64[offset / 8];
> > +case 4:
> > +return cxl_dstate->mbox_reg_state32[offset / 4];
>
> Numeric order seems more natural and I can't see a reason not to do it.
> + you do them in that order below.
>
> > +default:
> > +g_assert_not_reached();
> > +}
> > +}
> > +
> > +static void mailbox_mem_writel(uint32_t *reg_state, hwaddr offset,
> > + uint64_t value)
> > +{
> > +switch (offset) {
> > +case A_CXL_DEV_MAILBOX_CTRL:
> > +/* fallthrough */
> > +case A_CXL_DEV_MAILBOX_CAP:
> > +/* RO register */
> > +break;
> > +default:
> > +qemu_log_mask(LOG_UNIMP,
> > + "%s Unexpected 32-bit access to 0x%" PRIx64 "
> > (WI)\n",
> > + __func__, offset);
> > +break;
> > +}
> > +
> > +reg_state[offset / 4] = value;
> > +}
> > +
> > +static void mailbox_mem_writeq(uint64_t *reg_state, hwaddr offset,
> > + uint64_t value)
> > +{
> > +switch (offset) {
> > +case A_CXL_DEV_MAILBOX_CMD:
> > +break;
> > +case A_CXL_DEV_BG_CMD_STS:
> > +/* BG not supported */
> > +/* fallthrough */
> > +case A_CXL_DEV_MAILBOX_STS:
> > +/* Read only register, will get updated by the state machine */
> > +return;
> > +default:
> > +qemu_log_mask(LOG_UNIMP,
> > + "%s Unexpected 64-bit access to 0x%" PRIx64 "
> > (WI)\n",
> > + __func__, offset);
> > +return;
> > +}
> > +
> > +
> > +reg_state[offset / 8] = value;
> > +}
> > +
> > +static void mailbox_reg_write(void *opaque, hwaddr offset, uint64_t value,
> > + unsigned size)
> > +{
> > +CXLDeviceState *cxl_dstate = opaque;
> > +
> > +if (offset >= A_CXL_DEV_CMD_PAYLOAD) {
> > +memcpy(cxl_dstate->mbox_reg_state + offset, , size);
> > +return;
> > +}
> > +
> > +/*
> > + * Lock is needed to prevent concurrent writes as well as to prevent
> > writes
> > + * coming in while the firmware is processing. Without background
> > commands
> > + * or the second mailbox implemented, this serves no purpose since the
> > + * memory access is synchronized at a higher level (per memory region).
> > + */
> > +RCU_READ_LOCK_GUARD();
> > +
> > +switch (size) {
> > +case 4:
> > +mailbox_mem_writel(cxl_dstate->mbox_reg_state32, offset, value);
> > +break;
> > +case 8:
> > +mailbox_mem_writeq(cxl_dstate->mbox_reg_state64, offset, value);
> > +break;
> > +default:
> > +g_assert_not_reached();
> > +}
> > +
> > +if
Re: [RFC PATCH v3 05/31] hw/cxl/device: Implement basic mailbox (8.2.8.4)
On Mon, 1 Feb 2021 16:59:22 -0800
Ben Widawsky wrote:
> This is the beginning of implementing mailbox support for CXL 2.0
> devices. The implementation recognizes when the doorbell is rung,
> handles the command/payload, clears the doorbell while returning error
> codes and data.
>
> Generally the mailbox mechanism is designed to permit communication
> between the host OS and the firmware running on the device. For our
> purposes, we emulate both the firmware, implemented primarily in
> cxl-mailbox-utils.c, and the hardware.
>
> No commands are implemented yet.
>
> Signed-off-by: Ben Widawsky
> ---
> hw/cxl/cxl-device-utils.c | 125 ++-
> hw/cxl/cxl-mailbox-utils.c | 197
> hw/cxl/meson.build | 1 +
> include/hw/cxl/cxl.h| 3 +
> include/hw/cxl/cxl_device.h | 28 -
> 5 files changed, 349 insertions(+), 5 deletions(-)
> create mode 100644 hw/cxl/cxl-mailbox-utils.c
>
> diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> index bb15ad9a0f..6602606f3d 100644
> --- a/hw/cxl/cxl-device-utils.c
> +++ b/hw/cxl/cxl-device-utils.c
> @@ -40,6 +40,111 @@ static uint64_t dev_reg_read(void *opaque, hwaddr offset,
> unsigned size)
> return 0;
> }
>
> +static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +CXLDeviceState *cxl_dstate = opaque;
> +
> +switch (size) {
> +case 8:
> +return cxl_dstate->mbox_reg_state64[offset / 8];
> +case 4:
> +return cxl_dstate->mbox_reg_state32[offset / 4];
Numeric order seems more natural and I can't see a reason not to do it.
+ you do them in that order below.
> +default:
> +g_assert_not_reached();
> +}
> +}
> +
> +static void mailbox_mem_writel(uint32_t *reg_state, hwaddr offset,
> + uint64_t value)
> +{
> +switch (offset) {
> +case A_CXL_DEV_MAILBOX_CTRL:
> +/* fallthrough */
> +case A_CXL_DEV_MAILBOX_CAP:
> +/* RO register */
> +break;
> +default:
> +qemu_log_mask(LOG_UNIMP,
> + "%s Unexpected 32-bit access to 0x%" PRIx64 " (WI)\n",
> + __func__, offset);
> +break;
> +}
> +
> +reg_state[offset / 4] = value;
> +}
> +
> +static void mailbox_mem_writeq(uint64_t *reg_state, hwaddr offset,
> + uint64_t value)
> +{
> +switch (offset) {
> +case A_CXL_DEV_MAILBOX_CMD:
> +break;
> +case A_CXL_DEV_BG_CMD_STS:
> +/* BG not supported */
> +/* fallthrough */
> +case A_CXL_DEV_MAILBOX_STS:
> +/* Read only register, will get updated by the state machine */
> +return;
> +default:
> +qemu_log_mask(LOG_UNIMP,
> + "%s Unexpected 64-bit access to 0x%" PRIx64 " (WI)\n",
> + __func__, offset);
> +return;
> +}
> +
> +
> +reg_state[offset / 8] = value;
> +}
> +
> +static void mailbox_reg_write(void *opaque, hwaddr offset, uint64_t value,
> + unsigned size)
> +{
> +CXLDeviceState *cxl_dstate = opaque;
> +
> +if (offset >= A_CXL_DEV_CMD_PAYLOAD) {
> +memcpy(cxl_dstate->mbox_reg_state + offset, , size);
> +return;
> +}
> +
> +/*
> + * Lock is needed to prevent concurrent writes as well as to prevent
> writes
> + * coming in while the firmware is processing. Without background
> commands
> + * or the second mailbox implemented, this serves no purpose since the
> + * memory access is synchronized at a higher level (per memory region).
> + */
> +RCU_READ_LOCK_GUARD();
> +
> +switch (size) {
> +case 4:
> +mailbox_mem_writel(cxl_dstate->mbox_reg_state32, offset, value);
> +break;
> +case 8:
> +mailbox_mem_writeq(cxl_dstate->mbox_reg_state64, offset, value);
> +break;
> +default:
> +g_assert_not_reached();
> +}
> +
> +if (ARRAY_FIELD_EX32(cxl_dstate->mbox_reg_state32, CXL_DEV_MAILBOX_CTRL,
> + DOORBELL))
> +cxl_process_mailbox(cxl_dstate);
> +}
> +
> +static const MemoryRegionOps mailbox_ops = {
> +.read = mailbox_reg_read,
> +.write = mailbox_reg_write,
> +.endianness = DEVICE_LITTLE_ENDIAN,
> +.valid = {
> +.min_access_size = 1,
> +.max_access_size = 8,
> +.unaligned = false,
> +},
> +.impl = {
> +.min_access_size = 4,
> +.max_access_size = 8,
> +},
> +};
> +
> static const MemoryRegionOps dev_ops = {
> .read = dev_reg_read,
> .write = NULL, /* status register is read only */
> @@ -80,20 +185,33 @@ void cxl_device_register_block_init(Object *obj,
> CXLDeviceState *cxl_dstate)
>"cap-array", CXL_DEVICE_REGISTERS_OFFSET - 0);
> memory_region_init_io(_dstate->device, obj, _ops, cxl_dstate,
>"device-status",
[RFC PATCH v3 05/31] hw/cxl/device: Implement basic mailbox (8.2.8.4)
This is the beginning of implementing mailbox support for CXL 2.0
devices. The implementation recognizes when the doorbell is rung,
handles the command/payload, clears the doorbell while returning error
codes and data.
Generally the mailbox mechanism is designed to permit communication
between the host OS and the firmware running on the device. For our
purposes, we emulate both the firmware, implemented primarily in
cxl-mailbox-utils.c, and the hardware.
No commands are implemented yet.
Signed-off-by: Ben Widawsky
---
hw/cxl/cxl-device-utils.c | 125 ++-
hw/cxl/cxl-mailbox-utils.c | 197
hw/cxl/meson.build | 1 +
include/hw/cxl/cxl.h| 3 +
include/hw/cxl/cxl_device.h | 28 -
5 files changed, 349 insertions(+), 5 deletions(-)
create mode 100644 hw/cxl/cxl-mailbox-utils.c
diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
index bb15ad9a0f..6602606f3d 100644
--- a/hw/cxl/cxl-device-utils.c
+++ b/hw/cxl/cxl-device-utils.c
@@ -40,6 +40,111 @@ static uint64_t dev_reg_read(void *opaque, hwaddr offset,
unsigned size)
return 0;
}
+static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned size)
+{
+CXLDeviceState *cxl_dstate = opaque;
+
+switch (size) {
+case 8:
+return cxl_dstate->mbox_reg_state64[offset / 8];
+case 4:
+return cxl_dstate->mbox_reg_state32[offset / 4];
+default:
+g_assert_not_reached();
+}
+}
+
+static void mailbox_mem_writel(uint32_t *reg_state, hwaddr offset,
+ uint64_t value)
+{
+switch (offset) {
+case A_CXL_DEV_MAILBOX_CTRL:
+/* fallthrough */
+case A_CXL_DEV_MAILBOX_CAP:
+/* RO register */
+break;
+default:
+qemu_log_mask(LOG_UNIMP,
+ "%s Unexpected 32-bit access to 0x%" PRIx64 " (WI)\n",
+ __func__, offset);
+break;
+}
+
+reg_state[offset / 4] = value;
+}
+
+static void mailbox_mem_writeq(uint64_t *reg_state, hwaddr offset,
+ uint64_t value)
+{
+switch (offset) {
+case A_CXL_DEV_MAILBOX_CMD:
+break;
+case A_CXL_DEV_BG_CMD_STS:
+/* BG not supported */
+/* fallthrough */
+case A_CXL_DEV_MAILBOX_STS:
+/* Read only register, will get updated by the state machine */
+return;
+default:
+qemu_log_mask(LOG_UNIMP,
+ "%s Unexpected 64-bit access to 0x%" PRIx64 " (WI)\n",
+ __func__, offset);
+return;
+}
+
+
+reg_state[offset / 8] = value;
+}
+
+static void mailbox_reg_write(void *opaque, hwaddr offset, uint64_t value,
+ unsigned size)
+{
+CXLDeviceState *cxl_dstate = opaque;
+
+if (offset >= A_CXL_DEV_CMD_PAYLOAD) {
+memcpy(cxl_dstate->mbox_reg_state + offset, , size);
+return;
+}
+
+/*
+ * Lock is needed to prevent concurrent writes as well as to prevent writes
+ * coming in while the firmware is processing. Without background commands
+ * or the second mailbox implemented, this serves no purpose since the
+ * memory access is synchronized at a higher level (per memory region).
+ */
+RCU_READ_LOCK_GUARD();
+
+switch (size) {
+case 4:
+mailbox_mem_writel(cxl_dstate->mbox_reg_state32, offset, value);
+break;
+case 8:
+mailbox_mem_writeq(cxl_dstate->mbox_reg_state64, offset, value);
+break;
+default:
+g_assert_not_reached();
+}
+
+if (ARRAY_FIELD_EX32(cxl_dstate->mbox_reg_state32, CXL_DEV_MAILBOX_CTRL,
+ DOORBELL))
+cxl_process_mailbox(cxl_dstate);
+}
+
+static const MemoryRegionOps mailbox_ops = {
+.read = mailbox_reg_read,
+.write = mailbox_reg_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+.valid = {
+.min_access_size = 1,
+.max_access_size = 8,
+.unaligned = false,
+},
+.impl = {
+.min_access_size = 4,
+.max_access_size = 8,
+},
+};
+
static const MemoryRegionOps dev_ops = {
.read = dev_reg_read,
.write = NULL, /* status register is read only */
@@ -80,20 +185,33 @@ void cxl_device_register_block_init(Object *obj,
CXLDeviceState *cxl_dstate)
"cap-array", CXL_DEVICE_REGISTERS_OFFSET - 0);
memory_region_init_io(_dstate->device, obj, _ops, cxl_dstate,
"device-status", CXL_DEVICE_REGISTERS_LENGTH);
+memory_region_init_io(_dstate->mailbox, obj, _ops, cxl_dstate,
+ "mailbox", CXL_MAILBOX_REGISTERS_LENGTH);
memory_region_add_subregion(_dstate->device_registers, 0,
_dstate->caps);
memory_region_add_subregion(_dstate->device_registers,
CXL_DEVICE_REGISTERS_OFFSET,
_dstate->device);
+
