diff mbox series

[RFC,04/25] hw/cxl/device: Introduce a CXL device (8.2.8)

Message ID 20201111054724.794888-5-ben.widawsky@intel.com
State New
Headers show
Series Introduce CXL 2.0 Emulation | expand

Commit Message

Ben Widawsky Nov. 11, 2020, 5:47 a.m. UTC
A CXL device is a type of CXL component. Conceptually, a CXL device
would be a leaf node in a CXL topology. From an emulation perspective,
CXL devices are the most complex and so the actual implementation is
reserved for discrete commits.

This new device type is specifically catered towards the eventually
implementation of a Type3 CXL.mem device, 8.2.8.5 in the CXL 2.0
specification.

Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
---
 include/hw/cxl/cxl.h        |   1 +
 include/hw/cxl/cxl_device.h | 193 ++++++++++++++++++++++++++++++++++++
 2 files changed, 194 insertions(+)
 create mode 100644 include/hw/cxl/cxl_device.h

Comments

Jonathan Cameron Nov. 16, 2020, 1:07 p.m. UTC | #1
On Tue, 10 Nov 2020 21:47:03 -0800
Ben Widawsky <ben.widawsky@intel.com> wrote:

> A CXL device is a type of CXL component. Conceptually, a CXL device
> would be a leaf node in a CXL topology. From an emulation perspective,
> CXL devices are the most complex and so the actual implementation is
> reserved for discrete commits.
> 
> This new device type is specifically catered towards the eventually
> implementation of a Type3 CXL.mem device, 8.2.8.5 in the CXL 2.0
> specification.
> 
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>

As an RFC, would be good to have questions relavant to individual
patches if possible.  Makes it easier to know what you want feedback on.

The REG32 being used for 64 bit registers seems awkward. I'd suggest
we either break them up into DW and deal with the edge parts manually.

I'm not sure a REG64 definition would work due to lack of explicit alignment
guarantees.  Might be fine though.

One buglet inline and a few other comments.

Jonathan


> ---
>  include/hw/cxl/cxl.h        |   1 +
>  include/hw/cxl/cxl_device.h | 193 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 194 insertions(+)
>  create mode 100644 include/hw/cxl/cxl_device.h
> 
> diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
> index 55f6cc30a5..23f52c4cf9 100644
> --- a/include/hw/cxl/cxl.h
> +++ b/include/hw/cxl/cxl.h
> @@ -12,6 +12,7 @@
>  
>  #include "cxl_pci.h"
>  #include "cxl_component.h"
> +#include "cxl_device.h"
>  
>  #endif
>  
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> new file mode 100644
> index 0000000000..491eca6e05
> --- /dev/null
> +++ b/include/hw/cxl/cxl_device.h
> @@ -0,0 +1,193 @@
> +/*
> + * QEMU CXL Devices
> + *
> + * Copyright (c) 2020 Intel
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2. See the
> + * COPYING file in the top-level directory.
> + */
> +
> +#ifndef CXL_DEVICE_H
> +#define CXL_DEVICE_H
> +
> +#include "hw/register.h"
> +
> +/*
> + * The following is how a CXL device's MMIO space is laid out. The only
> + * requirement from the spec is that the capabilities array and the capability
> + * headers start at offset 0 and are contiguously packed. The headers themselves
> + * provide offsets to the register fields. For this emulation, registers will
> + * start at offset 0x80 (m == 0x80). No secondary mailbox is implemented which
> + * means that n = m + sizeof(mailbox registers) + sizeof(device registers).
> + *
> + * This is roughly described in 8.2.8 Figure 138 of the CXL 2.0 spec.
> + *
> + * n + PAYLOAD_SIZE_MAX  +---------------------------------+
> + *                       |                                 |
> + *                  ^    |                                 |
> + *                  |    |                                 |
> + *                  |    |                                 |
> + *                  |    |                                 |
> + *                  |    |         Command Payload         |
> + *                  |    |                                 |
> + *                  |    |                                 |
> + *                  |    |                                 |
> + *                  |    |                                 |
> + *                  |    |                                 |
> + *                  n    +---------------------------------+
> + *                  ^    |                                 |
> + *                  |    |    Device Capability Registers  |
> + *                  |    |    x, mailbox, y                |
> + *                  |    |                                 |
> + *                  m    +---------------------------------+
> + *                  ^    |     Device Capability Header y  |
> + *                  |    +---------------------------------+
> + *                  |    | Device Capability Header Mailbox|
> + *                  |    +------------- --------------------
> + *                  |    |     Device Capability Header x  |
> + *                  |    +---------------------------------+
> + *                  |    |                                 |
> + *                  |    |                                 |
> + *                  |    |      Device Cap Array[0..n]     |
> + *                  |    |                                 |
> + *                  |    |                                 |
> + *                  |    |                                 |
> + *                  0    +---------------------------------+
> + */
> +
> +#define CXL_DEVICE_CAP_HDR1_OFFSET 0x10 /* Figure 138 */
> +#define CXL_DEVICE_CAP_REG_SIZE 0x10 /* 8.2.8.2 */
> +
> +#define CXL_DEVICE_REGISTERS_OFFSET 0x80 /* Read comment above */
> +#define CXL_DEVICE_REGISTERS_LENGTH 0x8 /* 8.2.8.3.1 */
> +
> +#define CXL_MAILBOX_REGISTERS_OFFSET \
> +    (CXL_DEVICE_REGISTERS_OFFSET + CXL_DEVICE_REGISTERS_LENGTH)
> +#define CXL_MAILBOX_REGISTERS_SIZE 0x20
> +#define CXL_MAILBOX_PAYLOAD_SHIFT 11
> +#define CXL_MAILBOX_MAX_PAYLOAD_SIZE (1 << CXL_MAILBOX_PAYLOAD_SHIFT)
> +#define CXL_MAILBOX_REGISTERS_LENGTH \
> +    (CXL_MAILBOX_REGISTERS_SIZE + CXL_MAILBOX_MAX_PAYLOAD_SIZE)
> +
> +typedef struct cxl_device_state {
> +    /* Boss container and caps registers */
> +    MemoryRegion device_registers;
> +
> +    MemoryRegion caps;
> +    MemoryRegion device;
> +    MemoryRegion mailbox;
> +
> +    MemoryRegion *pmem;
> +    MemoryRegion *vmem;
> +
> +    bool active;
> +    uint16_t command;
> +    uint16_t payload_size;
> +    union {
> +        uint8_t caps_reg_state[CXL_DEVICE_CAP_REG_SIZE * 4]; /* ARRAY + 3 CAPS */
> +        uint32_t caps_reg_state32[0];
> +    };
> +} CXLDeviceState;
> +
> +/* Initialize the register block for a device */
> +void cxl_device_register_block_init(Object *obj, CXLDeviceState *dev);
> +
> +/* Set up default values for the register block */
> +void cxl_device_register_init_common(CXLDeviceState *dev);
> +
> +/* CXL 2.0 - 8.2.8.1 */
> +REG32(CXL_DEV_CAP_ARRAY, 0) /* 48b!?!?! */
> +    FIELD(CXL_DEV_CAP_ARRAY, CAP_ID, 0, 16)
> +    FIELD(CXL_DEV_CAP_ARRAY, CAP_VERSION, 16, 8)
> +REG32(CXL_DEV_CAP_ARRAY2, 4) /* We're going to pretend it's 64b */

Fair call.  Guess reserved 16 bits on the end will be the eventual fix.

> +    FIELD(CXL_DEV_CAP_ARRAY2, CAP_COUNT, 0, 16)
> +
> +/*
> + * In the 8.2.8.2, this is listed as a 128b register, but in 8.2.8, it says:
> + * > No registers defined in Section 8.2.8 are larger than 64-bits wide so that
> + * > is the maximum access size allowed for these registers. If this rule is not
> + * > followed, the behavior is undefined
> + *
> + * Here we've chosen to make it 4 dwords. The spec allows any pow2 multiple
> + * access to be used for a register (2 qwords, 8 words, 128 bytes).
> + *
> + * XXX: This is supposedly fixed for the release version of the spec. If this
> + * comment is still here, I've failed.

*sniggers in a sympathetic way*

> + */
> +#define CXL_DEVICE_CAPABILITY_HEADER_REGISTER(n, offset)                            \
> +    REG32(CXL_DEV_##n##_CAP_HDR0, offset)                 \
> +        FIELD(CXL_DEV_##n##_CAP_HDR0, CAP_ID, 0, 16)      \
> +        FIELD(CXL_DEV_##n##_CAP_HDR0, CAP_VERSION, 16, 8) \
> +    REG32(CXL_DEV_##n##_CAP_HDR1, offset + 4)             \
> +        FIELD(CXL_DEV_##n##_CAP_HDR1, CAP_OFFSET, 0, 32)  \
> +    REG32(CXL_DEV_##n##_CAP_HDR2, offset + 8)             \
> +        FIELD(CXL_DEV_##n##_CAP_HDR2, CAP_LENGTH, 0, 32)
> +
> +CXL_DEVICE_CAPABILITY_HEADER_REGISTER(DEVICE, CXL_DEVICE_CAP_HDR1_OFFSET)
> +CXL_DEVICE_CAPABILITY_HEADER_REGISTER(MAILBOX, CXL_DEVICE_CAP_HDR1_OFFSET + \
> +                                               CXL_DEVICE_CAP_REG_SIZE)
> +
> +REG32(CXL_DEV_MAILBOX_CAP, 0)
> +    FIELD(CXL_DEV_MAILBOX_CAP, PAYLOAD_SIZE, 0, 5)
> +    FIELD(CXL_DEV_MAILBOX_CAP, INT_CAP, 5, 1)
> +    FIELD(CXL_DEV_MAILBOX_CAP, BG_INT_CAP, 6, 1)
> +    FIELD(CXL_DEV_MAILBOX_CAP, MSI_N, 7, 4)
> +
> +REG32(CXL_DEV_MAILBOX_CTRL, 4)
> +    FIELD(CXL_DEV_MAILBOX_CTRL, DOORBELL, 0, 1)
> +    FIELD(CXL_DEV_MAILBOX_CTRL, INT_EN, 1, 2)

1 bit field, not 2.

> +    FIELD(CXL_DEV_MAILBOX_CTRL, BG_INT_EN, 2, 1)
> +
> +enum {
> +    CXL_CMD_EVENTS              = 0x1,

> +    CXL_CMD_IDENTIFY            = 0x40,
> +};
> +
> +REG32(CXL_DEV_MAILBOX_CMD, 8)
> +    FIELD(CXL_DEV_MAILBOX_CMD, OP, 0, 16)
> +    FIELD(CXL_DEV_MAILBOX_CMD, LENGTH, 16, 20)

That takes us out to bit 36 in a 32 bit register?
Probably needs a comment like the ones below.
Wouldn't want to miss fixing this one later.

> +
> +/* 8.2.8.4.5.1 Command Return Codes */
> +enum {
> +    RET_SUCCESS                 = 0x0,
> +    RET_BG_STARTED              = 0x1, /* Background Command Started */
> +    RET_EINVAL                  = 0x2, /* Invalid Input */
> +    RET_ENOTSUP                 = 0x3, /* Unsupported */
> +    RET_ENODEV                  = 0x4, /* Internal Error */

Mapping that to NODEV seems less than obvious.

> +    RET_ERESTART                = 0x5, /* Retry Required */
> +    RET_EBUSY                   = 0x6, /* Busy */
> +    RET_MEDIA_DISABLED          = 0x7, /* Media Disabled */
> +    RET_FW_EBUSY                = 0x8, /* FW Transfer in Progress */
> +    RET_FW_OOO                  = 0x9, /* FW Transfer Out of Order */
> +    RET_FW_AUTH                 = 0xa, /* FW Authentication Failed */
> +    RET_FW_EBADSLT              = 0xb, /* Invalid Slot */
> +    RET_FW_ROLLBACK             = 0xc, /* Activation Failed, FW Rolled Back */
> +    RET_FW_REBOOT               = 0xd, /* Activation Failed, Cold Reset Required */
> +    RET_ENOENT                  = 0xe, /* Invalid Handle */
> +    RET_EFAULT                  = 0xf, /* Invalid Physical Address */
> +    RET_POISON_E2BIG            = 0x10, /* Inject Poison Limit Reached */
> +    RET_EIO                     = 0x11, /* Permanent Media Failure */
> +    RET_ECANCELED               = 0x12, /* Aborted */
> +    RET_EACCESS                 = 0x13, /* Invalid Security State */
> +    RET_EPERM                   = 0x14, /* Incorrect Passphrase */
> +    RET_EPROTONOSUPPORT         = 0x15, /* Unsupported Mailbox */
> +    RET_EMSGSIZE                = 0x16, /* Invalid Payload Length */
> +    RET_MAX                     = 0x17
> +};
> +
> +/* XXX: actually a 64b register */
> +REG32(CXL_DEV_MAILBOX_STS, 0x10)
> +    FIELD(CXL_DEV_MAILBOX_STS, BG_OP, 0, 1)
> +    FIELD(CXL_DEV_MAILBOX_STS, ERRNO, 32, 16)
> +    FIELD(CXL_DEV_MAILBOX_STS, VENDOR_ERRNO, 48, 16)
> +
> +/* XXX: actually a 64b register */
> +REG32(CXL_DEV_BG_CMD_STS, 0x18)
> +    FIELD(CXL_DEV_BG_CMD_STS, BG, 0, 16)
> +    FIELD(CXL_DEV_BG_CMD_STS, DONE, 16, 7)
> +    FIELD(CXL_DEV_BG_CMD_STS, ERRNO, 32, 16)
> +    FIELD(CXL_DEV_BG_CMD_STS, VENDOR_ERRNO, 48, 16)
> +
> +REG32(CXL_DEV_CMD_PAYLOAD, 0x20)
> +
> +#endif
Ben Widawsky Nov. 16, 2020, 9:11 p.m. UTC | #2
On 20-11-16 13:07:56, Jonathan Cameron wrote:
> On Tue, 10 Nov 2020 21:47:03 -0800
> Ben Widawsky <ben.widawsky@intel.com> wrote:
> 
> > A CXL device is a type of CXL component. Conceptually, a CXL device
> > would be a leaf node in a CXL topology. From an emulation perspective,
> > CXL devices are the most complex and so the actual implementation is
> > reserved for discrete commits.
> > 
> > This new device type is specifically catered towards the eventually
> > implementation of a Type3 CXL.mem device, 8.2.8.5 in the CXL 2.0
> > specification.
> > 
> > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
> 
> As an RFC, would be good to have questions relavant to individual
> patches if possible.  Makes it easier to know what you want feedback on.
> 
> The REG32 being used for 64 bit registers seems awkward. I'd suggest
> we either break them up into DW and deal with the edge parts manually.
> 
> I'm not sure a REG64 definition would work due to lack of explicit alignment
> guarantees.  Might be fine though.

Agreed, although I think the current frequency with which I've had to do this,
and the XXX comments are decent, it's definitely a bit ugly. I found at least
two registers (I don't recall one, but the very important command register was
the other that you noticed below) which have a field that straddles the 32b
boundary. I think having to do an upper and lower field for that would kind of
stink.

Given that the codebase has gone on long enough without REG64, I didn't want to
poke that bear, although I had wired it up at some point.

So for now, I'd like to just leave these as they are.

> 
> One buglet inline and a few other comments.
> 
> Jonathan

Thanks. Anything not responded to is acknowledged and will hopefully make its
way into v2.

> 
> 
> > ---
> >  include/hw/cxl/cxl.h        |   1 +
> >  include/hw/cxl/cxl_device.h | 193 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 194 insertions(+)
> >  create mode 100644 include/hw/cxl/cxl_device.h
> > 
> > diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
> > index 55f6cc30a5..23f52c4cf9 100644
> > --- a/include/hw/cxl/cxl.h
> > +++ b/include/hw/cxl/cxl.h
> > @@ -12,6 +12,7 @@
> >  
> >  #include "cxl_pci.h"
> >  #include "cxl_component.h"
> > +#include "cxl_device.h"
> >  
> >  #endif
> >  
> > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> > new file mode 100644
> > index 0000000000..491eca6e05
> > --- /dev/null
> > +++ b/include/hw/cxl/cxl_device.h
> > @@ -0,0 +1,193 @@
> > +/*
> > + * QEMU CXL Devices
> > + *
> > + * Copyright (c) 2020 Intel
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2. See the
> > + * COPYING file in the top-level directory.
> > + */
> > +
> > +#ifndef CXL_DEVICE_H
> > +#define CXL_DEVICE_H
> > +
> > +#include "hw/register.h"
> > +
> > +/*
> > + * The following is how a CXL device's MMIO space is laid out. The only
> > + * requirement from the spec is that the capabilities array and the capability
> > + * headers start at offset 0 and are contiguously packed. The headers themselves
> > + * provide offsets to the register fields. For this emulation, registers will
> > + * start at offset 0x80 (m == 0x80). No secondary mailbox is implemented which
> > + * means that n = m + sizeof(mailbox registers) + sizeof(device registers).
> > + *
> > + * This is roughly described in 8.2.8 Figure 138 of the CXL 2.0 spec.
> > + *
> > + * n + PAYLOAD_SIZE_MAX  +---------------------------------+
> > + *                       |                                 |
> > + *                  ^    |                                 |
> > + *                  |    |                                 |
> > + *                  |    |                                 |
> > + *                  |    |                                 |
> > + *                  |    |         Command Payload         |
> > + *                  |    |                                 |
> > + *                  |    |                                 |
> > + *                  |    |                                 |
> > + *                  |    |                                 |
> > + *                  |    |                                 |
> > + *                  n    +---------------------------------+
> > + *                  ^    |                                 |
> > + *                  |    |    Device Capability Registers  |
> > + *                  |    |    x, mailbox, y                |
> > + *                  |    |                                 |
> > + *                  m    +---------------------------------+
> > + *                  ^    |     Device Capability Header y  |
> > + *                  |    +---------------------------------+
> > + *                  |    | Device Capability Header Mailbox|
> > + *                  |    +------------- --------------------
> > + *                  |    |     Device Capability Header x  |
> > + *                  |    +---------------------------------+
> > + *                  |    |                                 |
> > + *                  |    |                                 |
> > + *                  |    |      Device Cap Array[0..n]     |
> > + *                  |    |                                 |
> > + *                  |    |                                 |
> > + *                  |    |                                 |
> > + *                  0    +---------------------------------+
> > + */
> > +
> > +#define CXL_DEVICE_CAP_HDR1_OFFSET 0x10 /* Figure 138 */
> > +#define CXL_DEVICE_CAP_REG_SIZE 0x10 /* 8.2.8.2 */
> > +
> > +#define CXL_DEVICE_REGISTERS_OFFSET 0x80 /* Read comment above */
> > +#define CXL_DEVICE_REGISTERS_LENGTH 0x8 /* 8.2.8.3.1 */
> > +
> > +#define CXL_MAILBOX_REGISTERS_OFFSET \
> > +    (CXL_DEVICE_REGISTERS_OFFSET + CXL_DEVICE_REGISTERS_LENGTH)
> > +#define CXL_MAILBOX_REGISTERS_SIZE 0x20
> > +#define CXL_MAILBOX_PAYLOAD_SHIFT 11
> > +#define CXL_MAILBOX_MAX_PAYLOAD_SIZE (1 << CXL_MAILBOX_PAYLOAD_SHIFT)
> > +#define CXL_MAILBOX_REGISTERS_LENGTH \
> > +    (CXL_MAILBOX_REGISTERS_SIZE + CXL_MAILBOX_MAX_PAYLOAD_SIZE)
> > +
> > +typedef struct cxl_device_state {
> > +    /* Boss container and caps registers */
> > +    MemoryRegion device_registers;
> > +
> > +    MemoryRegion caps;
> > +    MemoryRegion device;
> > +    MemoryRegion mailbox;
> > +
> > +    MemoryRegion *pmem;
> > +    MemoryRegion *vmem;
> > +
> > +    bool active;
> > +    uint16_t command;
> > +    uint16_t payload_size;
> > +    union {
> > +        uint8_t caps_reg_state[CXL_DEVICE_CAP_REG_SIZE * 4]; /* ARRAY + 3 CAPS */
> > +        uint32_t caps_reg_state32[0];
> > +    };
> > +} CXLDeviceState;
> > +
> > +/* Initialize the register block for a device */
> > +void cxl_device_register_block_init(Object *obj, CXLDeviceState *dev);
> > +
> > +/* Set up default values for the register block */
> > +void cxl_device_register_init_common(CXLDeviceState *dev);
> > +
> > +/* CXL 2.0 - 8.2.8.1 */
> > +REG32(CXL_DEV_CAP_ARRAY, 0) /* 48b!?!?! */
> > +    FIELD(CXL_DEV_CAP_ARRAY, CAP_ID, 0, 16)
> > +    FIELD(CXL_DEV_CAP_ARRAY, CAP_VERSION, 16, 8)
> > +REG32(CXL_DEV_CAP_ARRAY2, 4) /* We're going to pretend it's 64b */
> 
> Fair call.  Guess reserved 16 bits on the end will be the eventual fix.

I reported it but I was too late to get it changed. Hopefully it will be fixed.

> 
> > +    FIELD(CXL_DEV_CAP_ARRAY2, CAP_COUNT, 0, 16)
> > +
> > +/*
> > + * In the 8.2.8.2, this is listed as a 128b register, but in 8.2.8, it says:
> > + * > No registers defined in Section 8.2.8 are larger than 64-bits wide so that
> > + * > is the maximum access size allowed for these registers. If this rule is not
> > + * > followed, the behavior is undefined
> > + *
> > + * Here we've chosen to make it 4 dwords. The spec allows any pow2 multiple
> > + * access to be used for a register (2 qwords, 8 words, 128 bytes).
> > + *
> > + * XXX: This is supposedly fixed for the release version of the spec. If this
> > + * comment is still here, I've failed.
> 
> *sniggers in a sympathetic way*

Notice that they didn't actually fix it in the release version of the spec, even
though I was assured they did!!!!

> 
> > + */
> > +#define CXL_DEVICE_CAPABILITY_HEADER_REGISTER(n, offset)                            \
> > +    REG32(CXL_DEV_##n##_CAP_HDR0, offset)                 \
> > +        FIELD(CXL_DEV_##n##_CAP_HDR0, CAP_ID, 0, 16)      \
> > +        FIELD(CXL_DEV_##n##_CAP_HDR0, CAP_VERSION, 16, 8) \
> > +    REG32(CXL_DEV_##n##_CAP_HDR1, offset + 4)             \
> > +        FIELD(CXL_DEV_##n##_CAP_HDR1, CAP_OFFSET, 0, 32)  \
> > +    REG32(CXL_DEV_##n##_CAP_HDR2, offset + 8)             \
> > +        FIELD(CXL_DEV_##n##_CAP_HDR2, CAP_LENGTH, 0, 32)
> > +
> > +CXL_DEVICE_CAPABILITY_HEADER_REGISTER(DEVICE, CXL_DEVICE_CAP_HDR1_OFFSET)
> > +CXL_DEVICE_CAPABILITY_HEADER_REGISTER(MAILBOX, CXL_DEVICE_CAP_HDR1_OFFSET + \
> > +                                               CXL_DEVICE_CAP_REG_SIZE)
> > +
> > +REG32(CXL_DEV_MAILBOX_CAP, 0)
> > +    FIELD(CXL_DEV_MAILBOX_CAP, PAYLOAD_SIZE, 0, 5)
> > +    FIELD(CXL_DEV_MAILBOX_CAP, INT_CAP, 5, 1)
> > +    FIELD(CXL_DEV_MAILBOX_CAP, BG_INT_CAP, 6, 1)
> > +    FIELD(CXL_DEV_MAILBOX_CAP, MSI_N, 7, 4)
> > +
> > +REG32(CXL_DEV_MAILBOX_CTRL, 4)
> > +    FIELD(CXL_DEV_MAILBOX_CTRL, DOORBELL, 0, 1)
> > +    FIELD(CXL_DEV_MAILBOX_CTRL, INT_EN, 1, 2)
> 
> 1 bit field, not 2.
> 
> > +    FIELD(CXL_DEV_MAILBOX_CTRL, BG_INT_EN, 2, 1)
> > +
> > +enum {
> > +    CXL_CMD_EVENTS              = 0x1,
> 
> > +    CXL_CMD_IDENTIFY            = 0x40,
> > +};
> > +
> > +REG32(CXL_DEV_MAILBOX_CMD, 8)
> > +    FIELD(CXL_DEV_MAILBOX_CMD, OP, 0, 16)
> > +    FIELD(CXL_DEV_MAILBOX_CMD, LENGTH, 16, 20)
> 
> That takes us out to bit 36 in a 32 bit register?
> Probably needs a comment like the ones below.
> Wouldn't want to miss fixing this one later.

I really thought I had a comment here... it's definitely needed.

> 
> > +
> > +/* 8.2.8.4.5.1 Command Return Codes */
> > +enum {
> > +    RET_SUCCESS                 = 0x0,
> > +    RET_BG_STARTED              = 0x1, /* Background Command Started */
> > +    RET_EINVAL                  = 0x2, /* Invalid Input */
> > +    RET_ENOTSUP                 = 0x3, /* Unsupported */
> > +    RET_ENODEV                  = 0x4, /* Internal Error */
> 
> Mapping that to NODEV seems less than obvious.

I tried to be cute and map as many things to errno as possible. Suggestions?

> 
> > +    RET_ERESTART                = 0x5, /* Retry Required */
> > +    RET_EBUSY                   = 0x6, /* Busy */
> > +    RET_MEDIA_DISABLED          = 0x7, /* Media Disabled */
> > +    RET_FW_EBUSY                = 0x8, /* FW Transfer in Progress */
> > +    RET_FW_OOO                  = 0x9, /* FW Transfer Out of Order */
> > +    RET_FW_AUTH                 = 0xa, /* FW Authentication Failed */
> > +    RET_FW_EBADSLT              = 0xb, /* Invalid Slot */
> > +    RET_FW_ROLLBACK             = 0xc, /* Activation Failed, FW Rolled Back */
> > +    RET_FW_REBOOT               = 0xd, /* Activation Failed, Cold Reset Required */
> > +    RET_ENOENT                  = 0xe, /* Invalid Handle */
> > +    RET_EFAULT                  = 0xf, /* Invalid Physical Address */
> > +    RET_POISON_E2BIG            = 0x10, /* Inject Poison Limit Reached */
> > +    RET_EIO                     = 0x11, /* Permanent Media Failure */
> > +    RET_ECANCELED               = 0x12, /* Aborted */
> > +    RET_EACCESS                 = 0x13, /* Invalid Security State */
> > +    RET_EPERM                   = 0x14, /* Incorrect Passphrase */
> > +    RET_EPROTONOSUPPORT         = 0x15, /* Unsupported Mailbox */
> > +    RET_EMSGSIZE                = 0x16, /* Invalid Payload Length */
> > +    RET_MAX                     = 0x17
> > +};
> > +
> > +/* XXX: actually a 64b register */
> > +REG32(CXL_DEV_MAILBOX_STS, 0x10)
> > +    FIELD(CXL_DEV_MAILBOX_STS, BG_OP, 0, 1)
> > +    FIELD(CXL_DEV_MAILBOX_STS, ERRNO, 32, 16)
> > +    FIELD(CXL_DEV_MAILBOX_STS, VENDOR_ERRNO, 48, 16)
> > +
> > +/* XXX: actually a 64b register */
> > +REG32(CXL_DEV_BG_CMD_STS, 0x18)
> > +    FIELD(CXL_DEV_BG_CMD_STS, BG, 0, 16)
> > +    FIELD(CXL_DEV_BG_CMD_STS, DONE, 16, 7)
> > +    FIELD(CXL_DEV_BG_CMD_STS, ERRNO, 32, 16)
> > +    FIELD(CXL_DEV_BG_CMD_STS, VENDOR_ERRNO, 48, 16)
> > +
> > +REG32(CXL_DEV_CMD_PAYLOAD, 0x20)
> > +
> > +#endif
>
Jonathan Cameron Nov. 17, 2020, 2:21 p.m. UTC | #3
On Mon, 16 Nov 2020 13:11:16 -0800
Ben Widawsky <ben.widawsky@intel.com> wrote:

> On 20-11-16 13:07:56, Jonathan Cameron wrote:
> > On Tue, 10 Nov 2020 21:47:03 -0800
> > Ben Widawsky <ben.widawsky@intel.com> wrote:
> >   
> > > A CXL device is a type of CXL component. Conceptually, a CXL device
> > > would be a leaf node in a CXL topology. From an emulation perspective,
> > > CXL devices are the most complex and so the actual implementation is
> > > reserved for discrete commits.
> > > 
> > > This new device type is specifically catered towards the eventually
> > > implementation of a Type3 CXL.mem device, 8.2.8.5 in the CXL 2.0
> > > specification.
> > > 
> > > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>  
> > 
> > As an RFC, would be good to have questions relavant to individual
> > patches if possible.  Makes it easier to know what you want feedback on.
> > 
> > The REG32 being used for 64 bit registers seems awkward. I'd suggest
> > we either break them up into DW and deal with the edge parts manually.
> > 
> > I'm not sure a REG64 definition would work due to lack of explicit alignment
> > guarantees.  Might be fine though.  
> 
> Agreed, although I think the current frequency with which I've had to do this,
> and the XXX comments are decent, it's definitely a bit ugly. I found at least
> two registers (I don't recall one, but the very important command register was
> the other that you noticed below) which have a field that straddles the 32b
> boundary. I think having to do an upper and lower field for that would kind of
> stink.
> 
> Given that the codebase has gone on long enough without REG64, I didn't want to
> poke that bear, although I had wired it up at some point.
> 
> So for now, I'd like to just leave these as they are.
> 
> > 
> > One buglet inline and a few other comments.
> > 
> > Jonathan  
> 
> Thanks. Anything not responded to is acknowledged and will hopefully make its
> way into v2.
> 

...

> 
> >   
> > > +
> > > +/* 8.2.8.4.5.1 Command Return Codes */
> > > +enum {
> > > +    RET_SUCCESS                 = 0x0,
> > > +    RET_BG_STARTED              = 0x1, /* Background Command Started */
> > > +    RET_EINVAL                  = 0x2, /* Invalid Input */
> > > +    RET_ENOTSUP                 = 0x3, /* Unsupported */
> > > +    RET_ENODEV                  = 0x4, /* Internal Error */  
> > 
> > Mapping that to NODEV seems less than obvious.  
> 
> I tried to be cute and map as many things to errno as possible. Suggestions?

Don't bother being cute? :)
More seriously, I'd carry them as matching the spec out until you actually
have to return a standard error.   Fine to have a conversion function
that does a best possible mapping though so as to keep things consistent
across multiple locations.  Mind you perhaps qemu has a standard idiom for this?

cxl_cmd_ret_to_errno()


> 
> >   
> > > +    RET_ERESTART                = 0x5, /* Retry Required */
> > > +    RET_EBUSY                   = 0x6, /* Busy */
> > > +    RET_MEDIA_DISABLED          = 0x7, /* Media Disabled */
> > > +    RET_FW_EBUSY                = 0x8, /* FW Transfer in Progress */
> > > +    RET_FW_OOO                  = 0x9, /* FW Transfer Out of Order */
> > > +    RET_FW_AUTH                 = 0xa, /* FW Authentication Failed */
> > > +    RET_FW_EBADSLT              = 0xb, /* Invalid Slot */
> > > +    RET_FW_ROLLBACK             = 0xc, /* Activation Failed, FW Rolled Back */
> > > +    RET_FW_REBOOT               = 0xd, /* Activation Failed, Cold Reset Required */
> > > +    RET_ENOENT                  = 0xe, /* Invalid Handle */
> > > +    RET_EFAULT                  = 0xf, /* Invalid Physical Address */
> > > +    RET_POISON_E2BIG            = 0x10, /* Inject Poison Limit Reached */
> > > +    RET_EIO                     = 0x11, /* Permanent Media Failure */
> > > +    RET_ECANCELED               = 0x12, /* Aborted */
> > > +    RET_EACCESS                 = 0x13, /* Invalid Security State */
> > > +    RET_EPERM                   = 0x14, /* Incorrect Passphrase */
> > > +    RET_EPROTONOSUPPORT         = 0x15, /* Unsupported Mailbox */
> > > +    RET_EMSGSIZE                = 0x16, /* Invalid Payload Length */
> > > +    RET_MAX                     = 0x17
> > > +};
> > > +
> > > +/* XXX: actually a 64b register */
> > > +REG32(CXL_DEV_MAILBOX_STS, 0x10)
> > > +    FIELD(CXL_DEV_MAILBOX_STS, BG_OP, 0, 1)
> > > +    FIELD(CXL_DEV_MAILBOX_STS, ERRNO, 32, 16)
> > > +    FIELD(CXL_DEV_MAILBOX_STS, VENDOR_ERRNO, 48, 16)
> > > +
> > > +/* XXX: actually a 64b register */
> > > +REG32(CXL_DEV_BG_CMD_STS, 0x18)
> > > +    FIELD(CXL_DEV_BG_CMD_STS, BG, 0, 16)
> > > +    FIELD(CXL_DEV_BG_CMD_STS, DONE, 16, 7)
> > > +    FIELD(CXL_DEV_BG_CMD_STS, ERRNO, 32, 16)
> > > +    FIELD(CXL_DEV_BG_CMD_STS, VENDOR_ERRNO, 48, 16)
> > > +
> > > +REG32(CXL_DEV_CMD_PAYLOAD, 0x20)
> > > +
> > > +#endif  
> >
diff mbox series

Patch

diff --git a/include/hw/cxl/cxl.h b/include/hw/cxl/cxl.h
index 55f6cc30a5..23f52c4cf9 100644
--- a/include/hw/cxl/cxl.h
+++ b/include/hw/cxl/cxl.h
@@ -12,6 +12,7 @@ 
 
 #include "cxl_pci.h"
 #include "cxl_component.h"
+#include "cxl_device.h"
 
 #endif
 
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
new file mode 100644
index 0000000000..491eca6e05
--- /dev/null
+++ b/include/hw/cxl/cxl_device.h
@@ -0,0 +1,193 @@ 
+/*
+ * QEMU CXL Devices
+ *
+ * Copyright (c) 2020 Intel
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See the
+ * COPYING file in the top-level directory.
+ */
+
+#ifndef CXL_DEVICE_H
+#define CXL_DEVICE_H
+
+#include "hw/register.h"
+
+/*
+ * The following is how a CXL device's MMIO space is laid out. The only
+ * requirement from the spec is that the capabilities array and the capability
+ * headers start at offset 0 and are contiguously packed. The headers themselves
+ * provide offsets to the register fields. For this emulation, registers will
+ * start at offset 0x80 (m == 0x80). No secondary mailbox is implemented which
+ * means that n = m + sizeof(mailbox registers) + sizeof(device registers).
+ *
+ * This is roughly described in 8.2.8 Figure 138 of the CXL 2.0 spec.
+ *
+ * n + PAYLOAD_SIZE_MAX  +---------------------------------+
+ *                       |                                 |
+ *                  ^    |                                 |
+ *                  |    |                                 |
+ *                  |    |                                 |
+ *                  |    |                                 |
+ *                  |    |         Command Payload         |
+ *                  |    |                                 |
+ *                  |    |                                 |
+ *                  |    |                                 |
+ *                  |    |                                 |
+ *                  |    |                                 |
+ *                  n    +---------------------------------+
+ *                  ^    |                                 |
+ *                  |    |    Device Capability Registers  |
+ *                  |    |    x, mailbox, y                |
+ *                  |    |                                 |
+ *                  m    +---------------------------------+
+ *                  ^    |     Device Capability Header y  |
+ *                  |    +---------------------------------+
+ *                  |    | Device Capability Header Mailbox|
+ *                  |    +------------- --------------------
+ *                  |    |     Device Capability Header x  |
+ *                  |    +---------------------------------+
+ *                  |    |                                 |
+ *                  |    |                                 |
+ *                  |    |      Device Cap Array[0..n]     |
+ *                  |    |                                 |
+ *                  |    |                                 |
+ *                  |    |                                 |
+ *                  0    +---------------------------------+
+ */
+
+#define CXL_DEVICE_CAP_HDR1_OFFSET 0x10 /* Figure 138 */
+#define CXL_DEVICE_CAP_REG_SIZE 0x10 /* 8.2.8.2 */
+
+#define CXL_DEVICE_REGISTERS_OFFSET 0x80 /* Read comment above */
+#define CXL_DEVICE_REGISTERS_LENGTH 0x8 /* 8.2.8.3.1 */
+
+#define CXL_MAILBOX_REGISTERS_OFFSET \
+    (CXL_DEVICE_REGISTERS_OFFSET + CXL_DEVICE_REGISTERS_LENGTH)
+#define CXL_MAILBOX_REGISTERS_SIZE 0x20
+#define CXL_MAILBOX_PAYLOAD_SHIFT 11
+#define CXL_MAILBOX_MAX_PAYLOAD_SIZE (1 << CXL_MAILBOX_PAYLOAD_SHIFT)
+#define CXL_MAILBOX_REGISTERS_LENGTH \
+    (CXL_MAILBOX_REGISTERS_SIZE + CXL_MAILBOX_MAX_PAYLOAD_SIZE)
+
+typedef struct cxl_device_state {
+    /* Boss container and caps registers */
+    MemoryRegion device_registers;
+
+    MemoryRegion caps;
+    MemoryRegion device;
+    MemoryRegion mailbox;
+
+    MemoryRegion *pmem;
+    MemoryRegion *vmem;
+
+    bool active;
+    uint16_t command;
+    uint16_t payload_size;
+    union {
+        uint8_t caps_reg_state[CXL_DEVICE_CAP_REG_SIZE * 4]; /* ARRAY + 3 CAPS */
+        uint32_t caps_reg_state32[0];
+    };
+} CXLDeviceState;
+
+/* Initialize the register block for a device */
+void cxl_device_register_block_init(Object *obj, CXLDeviceState *dev);
+
+/* Set up default values for the register block */
+void cxl_device_register_init_common(CXLDeviceState *dev);
+
+/* CXL 2.0 - 8.2.8.1 */
+REG32(CXL_DEV_CAP_ARRAY, 0) /* 48b!?!?! */
+    FIELD(CXL_DEV_CAP_ARRAY, CAP_ID, 0, 16)
+    FIELD(CXL_DEV_CAP_ARRAY, CAP_VERSION, 16, 8)
+REG32(CXL_DEV_CAP_ARRAY2, 4) /* We're going to pretend it's 64b */
+    FIELD(CXL_DEV_CAP_ARRAY2, CAP_COUNT, 0, 16)
+
+/*
+ * In the 8.2.8.2, this is listed as a 128b register, but in 8.2.8, it says:
+ * > No registers defined in Section 8.2.8 are larger than 64-bits wide so that
+ * > is the maximum access size allowed for these registers. If this rule is not
+ * > followed, the behavior is undefined
+ *
+ * Here we've chosen to make it 4 dwords. The spec allows any pow2 multiple
+ * access to be used for a register (2 qwords, 8 words, 128 bytes).
+ *
+ * XXX: This is supposedly fixed for the release version of the spec. If this
+ * comment is still here, I've failed.
+ */
+#define CXL_DEVICE_CAPABILITY_HEADER_REGISTER(n, offset)                            \
+    REG32(CXL_DEV_##n##_CAP_HDR0, offset)                 \
+        FIELD(CXL_DEV_##n##_CAP_HDR0, CAP_ID, 0, 16)      \
+        FIELD(CXL_DEV_##n##_CAP_HDR0, CAP_VERSION, 16, 8) \
+    REG32(CXL_DEV_##n##_CAP_HDR1, offset + 4)             \
+        FIELD(CXL_DEV_##n##_CAP_HDR1, CAP_OFFSET, 0, 32)  \
+    REG32(CXL_DEV_##n##_CAP_HDR2, offset + 8)             \
+        FIELD(CXL_DEV_##n##_CAP_HDR2, CAP_LENGTH, 0, 32)
+
+CXL_DEVICE_CAPABILITY_HEADER_REGISTER(DEVICE, CXL_DEVICE_CAP_HDR1_OFFSET)
+CXL_DEVICE_CAPABILITY_HEADER_REGISTER(MAILBOX, CXL_DEVICE_CAP_HDR1_OFFSET + \
+                                               CXL_DEVICE_CAP_REG_SIZE)
+
+REG32(CXL_DEV_MAILBOX_CAP, 0)
+    FIELD(CXL_DEV_MAILBOX_CAP, PAYLOAD_SIZE, 0, 5)
+    FIELD(CXL_DEV_MAILBOX_CAP, INT_CAP, 5, 1)
+    FIELD(CXL_DEV_MAILBOX_CAP, BG_INT_CAP, 6, 1)
+    FIELD(CXL_DEV_MAILBOX_CAP, MSI_N, 7, 4)
+
+REG32(CXL_DEV_MAILBOX_CTRL, 4)
+    FIELD(CXL_DEV_MAILBOX_CTRL, DOORBELL, 0, 1)
+    FIELD(CXL_DEV_MAILBOX_CTRL, INT_EN, 1, 2)
+    FIELD(CXL_DEV_MAILBOX_CTRL, BG_INT_EN, 2, 1)
+
+enum {
+    CXL_CMD_EVENTS              = 0x1,
+    CXL_CMD_IDENTIFY            = 0x40,
+};
+
+REG32(CXL_DEV_MAILBOX_CMD, 8)
+    FIELD(CXL_DEV_MAILBOX_CMD, OP, 0, 16)
+    FIELD(CXL_DEV_MAILBOX_CMD, LENGTH, 16, 20)
+
+/* 8.2.8.4.5.1 Command Return Codes */
+enum {
+    RET_SUCCESS                 = 0x0,
+    RET_BG_STARTED              = 0x1, /* Background Command Started */
+    RET_EINVAL                  = 0x2, /* Invalid Input */
+    RET_ENOTSUP                 = 0x3, /* Unsupported */
+    RET_ENODEV                  = 0x4, /* Internal Error */
+    RET_ERESTART                = 0x5, /* Retry Required */
+    RET_EBUSY                   = 0x6, /* Busy */
+    RET_MEDIA_DISABLED          = 0x7, /* Media Disabled */
+    RET_FW_EBUSY                = 0x8, /* FW Transfer in Progress */
+    RET_FW_OOO                  = 0x9, /* FW Transfer Out of Order */
+    RET_FW_AUTH                 = 0xa, /* FW Authentication Failed */
+    RET_FW_EBADSLT              = 0xb, /* Invalid Slot */
+    RET_FW_ROLLBACK             = 0xc, /* Activation Failed, FW Rolled Back */
+    RET_FW_REBOOT               = 0xd, /* Activation Failed, Cold Reset Required */
+    RET_ENOENT                  = 0xe, /* Invalid Handle */
+    RET_EFAULT                  = 0xf, /* Invalid Physical Address */
+    RET_POISON_E2BIG            = 0x10, /* Inject Poison Limit Reached */
+    RET_EIO                     = 0x11, /* Permanent Media Failure */
+    RET_ECANCELED               = 0x12, /* Aborted */
+    RET_EACCESS                 = 0x13, /* Invalid Security State */
+    RET_EPERM                   = 0x14, /* Incorrect Passphrase */
+    RET_EPROTONOSUPPORT         = 0x15, /* Unsupported Mailbox */
+    RET_EMSGSIZE                = 0x16, /* Invalid Payload Length */
+    RET_MAX                     = 0x17
+};
+
+/* XXX: actually a 64b register */
+REG32(CXL_DEV_MAILBOX_STS, 0x10)
+    FIELD(CXL_DEV_MAILBOX_STS, BG_OP, 0, 1)
+    FIELD(CXL_DEV_MAILBOX_STS, ERRNO, 32, 16)
+    FIELD(CXL_DEV_MAILBOX_STS, VENDOR_ERRNO, 48, 16)
+
+/* XXX: actually a 64b register */
+REG32(CXL_DEV_BG_CMD_STS, 0x18)
+    FIELD(CXL_DEV_BG_CMD_STS, BG, 0, 16)
+    FIELD(CXL_DEV_BG_CMD_STS, DONE, 16, 7)
+    FIELD(CXL_DEV_BG_CMD_STS, ERRNO, 32, 16)
+    FIELD(CXL_DEV_BG_CMD_STS, VENDOR_ERRNO, 48, 16)
+
+REG32(CXL_DEV_CMD_PAYLOAD, 0x20)
+
+#endif