From: Ben Widawsky <ben.widawsky@intel.com>
To: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Cc: Vishal Verma <vishal.l.verma@intel.com>,
Dan Williams <dan.j.williams@intel.com>,
qemu-devel@nongnu.org
Subject: Re: [RFC PATCH 06/25] hw/cxl/device: Add device status (8.2.8.3)
Date: Mon, 16 Nov 2020 13:18:41 -0800 [thread overview]
Message-ID: <20201116211841.y6ihb7i7ywrwsk2f@intel.com> (raw)
In-Reply-To: <20201116131608.000052e6@Huawei.com>
On 20-11-16 13:16:08, Jonathan Cameron wrote:
> On Tue, 10 Nov 2020 21:47:05 -0800
> Ben Widawsky <ben.widawsky@intel.com> wrote:
>
> > This implements the CXL device status registers from 8.2.8.3.1 in the
> > CXL 2.0 specification. It is capability ID 0001h.
> >
> > Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
>
> It does some other stuff it shouldn't as well. Please tidy that up before
> v2. A few other passing comments inline.
>
> Thanks,
>
> Jonathan
>
>
> > ---
> > hw/cxl/cxl-device-utils.c | 45 +++++++++++++++++++++++++++++++++-
> > include/hw/cxl/cxl_device.h | 49 ++++++++++++-------------------------
> > 2 files changed, 60 insertions(+), 34 deletions(-)
> >
> > diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> > index a391bb15c6..78144e103c 100644
> > --- a/hw/cxl/cxl-device-utils.c
> > +++ b/hw/cxl/cxl-device-utils.c
> > @@ -33,6 +33,42 @@ static uint64_t caps_reg_read(void *opaque, hwaddr offset, unsigned size)
> > return ldn_le_p(cxl_dstate->caps_reg_state + offset, size);
> > }
> >
> > +static uint64_t dev_reg_read(void *opaque, hwaddr offset, unsigned size)
> > +{
> > + uint64_t retval = 0;
>
> Doesn't seem to be used.
>
It's required for ldn_le_p, or did you mean something else?
> > +
>
> Perhaps break the alignment check out to a utility function given this sanity check
> is same as in previous patch.
>
> > + switch (size) {
> > + case 4:
> > + if (unlikely(offset & (sizeof(uint32_t) - 1))) {
> > + qemu_log_mask(LOG_UNIMP, "Unaligned register read\n");
> > + return 0;
> > + }
> > + break;
> > + case 8:
> > + if (unlikely(offset & (sizeof(uint64_t) - 1))) {
> > + qemu_log_mask(LOG_UNIMP, "Unaligned register read\n");
> > + return 0;
> > + }
> > + break;
> > + }
> > +
> > + return ldn_le_p(&retval, size);
> > +}
> > +
> > +static const MemoryRegionOps dev_ops = {
> > + .read = dev_reg_read,
> > + .write = NULL,
> > + .endianness = DEVICE_LITTLE_ENDIAN,
> > + .valid = {
> > + .min_access_size = 4,
> > + .max_access_size = 8,
> > + },
> > + .impl = {
> > + .min_access_size = 4,
> > + .max_access_size = 8,
> > + },
> > +};
> > +
> > static const MemoryRegionOps caps_ops = {
> > .read = caps_reg_read,
> > .write = NULL,
> > @@ -56,18 +92,25 @@ void cxl_device_register_block_init(Object *obj, CXLDeviceState *cxl_dstate)
> >
> > memory_region_init_io(&cxl_dstate->caps, obj, &caps_ops, cxl_dstate,
> > "cap-array", CXL_DEVICE_REGISTERS_OFFSET - 0);
> > + memory_region_init_io(&cxl_dstate->device, obj, &dev_ops, cxl_dstate,
> > + "device-status", CXL_DEVICE_REGISTERS_LENGTH);
> >
> > memory_region_add_subregion(&cxl_dstate->device_registers, 0,
> > &cxl_dstate->caps);
> > + memory_region_add_subregion(&cxl_dstate->device_registers,
> > + CXL_DEVICE_REGISTERS_OFFSET,
> > + &cxl_dstate->device);
> > }
> >
> > void cxl_device_register_init_common(CXLDeviceState *cxl_dstate)
> > {
> > uint32_t *cap_hdrs = cxl_dstate->caps_reg_state32;
> > - const int cap_count = 0;
> > + const int cap_count = 1;
> >
> > /* CXL Device Capabilities Array Register */
> > ARRAY_FIELD_DP32(cap_hdrs, CXL_DEV_CAP_ARRAY, CAP_ID, 0);
> > ARRAY_FIELD_DP32(cap_hdrs, CXL_DEV_CAP_ARRAY, CAP_VERSION, 1);
> > ARRAY_FIELD_DP32(cap_hdrs, CXL_DEV_CAP_ARRAY2, CAP_COUNT, cap_count);
> > +
> > + cxl_device_cap_init(cxl_dstate, DEVICE, 1);
> > }
> > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> > index 491eca6e05..2c674fdc9c 100644
> > --- a/include/hw/cxl/cxl_device.h
> > +++ b/include/hw/cxl/cxl_device.h
> > @@ -127,6 +127,22 @@ 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)
> >
> > +#define cxl_device_cap_init(dstate, reg, cap_id) \
> > + do { \
> > + uint32_t *cap_hdrs = dstate->caps_reg_state32; \
> > + int which = R_CXL_DEV_##reg##_CAP_HDR0; \
> > + cap_hdrs[which] = \
> > + FIELD_DP32(cap_hdrs[which], CXL_DEV_##reg##_CAP_HDR0, CAP_ID, cap_id); \
> > + cap_hdrs[which] = FIELD_DP32( \
> > + cap_hdrs[which], CXL_DEV_##reg##_CAP_HDR0, CAP_VERSION, 1); \
> > + cap_hdrs[which + 1] = \
> > + FIELD_DP32(cap_hdrs[which + 1], CXL_DEV_##reg##_CAP_HDR1, \
> > + CAP_OFFSET, CXL_##reg##_REGISTERS_OFFSET); \
> > + cap_hdrs[which + 2] = \
> > + FIELD_DP32(cap_hdrs[which + 2], CXL_DEV_##reg##_CAP_HDR2, \
> > + CAP_LENGTH, CXL_##reg##_REGISTERS_LENGTH); \
> > + } while (0)
> > +
> > REG32(CXL_DEV_MAILBOX_CAP, 0)
> > FIELD(CXL_DEV_MAILBOX_CAP, PAYLOAD_SIZE, 0, 5)
> > FIELD(CXL_DEV_MAILBOX_CAP, INT_CAP, 5, 1)
> > @@ -138,43 +154,10 @@ REG32(CXL_DEV_MAILBOX_CTRL, 4)
> > 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 */
>
> Umm. We only just introduced this a few patches ago. Please tidy that
> up so we don't end up bringing things in and out again.
>
> > -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)
>
next prev parent reply other threads:[~2020-11-16 21:19 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-11 5:46 [RFC PATCH 00/25] Introduce CXL 2.0 Emulation Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 01/25] Temp: Add the PCI_EXT_ID_DVSEC definition to the qemu pci_regs.h copy Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 02/25] hw/pci/cxl: Add a CXL component type (interface) Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 03/25] hw/cxl/component: Introduce CXL components (8.1.x, 8.2.5) Ben Widawsky
2020-11-16 12:03 ` Jonathan Cameron
2020-11-16 19:19 ` Ben Widawsky
2020-11-17 12:29 ` Jonathan Cameron
2020-11-24 23:09 ` Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 04/25] hw/cxl/device: Introduce a CXL device (8.2.8) Ben Widawsky
2020-11-16 13:07 ` Jonathan Cameron
2020-11-16 21:11 ` Ben Widawsky
2020-11-17 14:21 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 05/25] hw/cxl/device: Implement the CAP array (8.2.8.1-2) Ben Widawsky
2020-11-16 13:11 ` Jonathan Cameron
2020-11-16 18:08 ` Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 06/25] hw/cxl/device: Add device status (8.2.8.3) Ben Widawsky
2020-11-16 13:16 ` Jonathan Cameron
2020-11-16 21:18 ` Ben Widawsky [this message]
2020-11-17 14:24 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 07/25] hw/cxl/device: Implement basic mailbox (8.2.8.4) Ben Widawsky
2020-11-16 13:46 ` Jonathan Cameron
2020-11-16 21:42 ` Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 08/25] hw/cxl/device: Add memory devices (8.2.8.5) Ben Widawsky
2020-11-16 16:37 ` Jonathan Cameron
2020-11-16 21:45 ` Ben Widawsky
2020-11-17 14:31 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 09/25] hw/pxb: Use a type for realizing expanders Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 10/25] hw/pci/cxl: Create a CXL bus type Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 11/25] hw/pxb: Allow creation of a CXL PXB (host bridge) Ben Widawsky
2020-11-16 16:44 ` Jonathan Cameron
2020-11-16 22:01 ` Ben Widawsky
2020-11-17 14:33 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 12/25] acpi/pci: Consolidate host bridge setup Ben Widawsky
2020-11-12 17:46 ` Ben Widawsky
2020-11-16 16:45 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 13/25] hw/pci: Plumb _UID through host bridges Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 14/25] hw/cxl/component: Implement host bridge MMIO (8.2.5, table 142) Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 15/25] acpi/pxb/cxl: Reserve host bridge MMIO Ben Widawsky
2020-11-16 16:54 ` Jonathan Cameron
2020-11-11 5:47 ` [RFC PATCH 16/25] hw/pxb/cxl: Add "windows" for host bridges Ben Widawsky
2020-11-13 0:49 ` Ben Widawsky
2020-11-23 19:12 ` Philippe Mathieu-Daudé
2020-11-11 5:47 ` [RFC PATCH 17/25] hw/cxl/rp: Add a root port Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 18/25] hw/cxl/device: Add a memory device (8.2.8.5) Ben Widawsky
2020-11-12 18:37 ` Eric Blake
2020-11-13 7:47 ` Markus Armbruster
2020-11-25 16:53 ` Ben Widawsky
2020-11-26 6:36 ` Markus Armbruster
2020-11-30 17:07 ` Ben Widawsky
2020-12-01 17:06 ` Markus Armbruster
2020-11-11 5:47 ` [RFC PATCH 19/25] hw/cxl/device: Implement MMIO HDM decoding (8.2.5.12) Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 20/25] acpi/cxl: Add _OSC implementation (9.14.2) Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 21/25] acpi/cxl: Introduce a compat-driver UUID for CXL _OSC Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 22/25] acpi/cxl: Create the CEDT (9.14.1) Ben Widawsky
2020-11-16 17:15 ` Jonathan Cameron
2020-11-16 22:05 ` Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 23/25] Temp: acpi/cxl: Add ACPI0017 (CEDT awareness) Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 24/25] WIP: i386/cxl: Initialize a host bridge Ben Widawsky
2020-11-11 5:47 ` [RFC PATCH 25/25] qtest/cxl: Add very basic sanity tests Ben Widawsky
2020-11-16 17:21 ` [RFC PATCH 00/25] Introduce CXL 2.0 Emulation Jonathan Cameron
2020-11-16 18:06 ` Ben Widawsky
2020-11-17 14:09 ` Jonathan Cameron
2020-11-25 18:29 ` Ben Widawsky
2020-12-04 14:27 ` Daniel P. Berrangé
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201116211841.y6ihb7i7ywrwsk2f@intel.com \
--to=ben.widawsky@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=dan.j.williams@intel.com \
--cc=qemu-devel@nongnu.org \
--cc=vishal.l.verma@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).