Re: [PATCH v6 cxl2.0-v6-doe 4/6] cxl/compliance: CXL Compliance Data Object Exchange implementation

2021-06-15 Thread Jonathan Cameron
On Thu, 10 Jun 2021 09:16:20 -0400
Chris Browy  wrote:

> From: hchkuo 
> 
> The Data Object Exchange implementation of CXL Compliance Mode is
> referring to "Compute Express Link (CXL) Specification, Rev. 2.0, Oct.
> 2020".
> 
> The data structure of CXL compliance request and response is added to
> the header. Due to the scope limitation of QEMU, most of the compliance
> response is limited to returning corresponding length.
> 
> A DOE capability of CXL Compliance is added to hw/mem/cxl_type3.c with
> capability offset 0x160. The config read/write to this capability range
> can be generated in the OS to request the Compliance info.
> 
> Signed-off-by: hchkuo 
> Signed-off-by: Chris Browy 

After too much time staring at the spec, I have a few queries on this one.
Some we can't discuss here for all the normal reasons ;)

> ---
>  hw/mem/cxl_type3.c  | 147 
>  include/hw/cxl/cxl_compliance.h | 293 
>  include/hw/cxl/cxl_component.h  |   3 +
>  include/hw/cxl/cxl_device.h |   3 +
>  include/hw/cxl/cxl_pci.h|   1 +
>  5 files changed, 447 insertions(+)
>  create mode 100644 include/hw/cxl/cxl_compliance.h
> 
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index bf33ddb915..569872eb36 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -13,6 +13,134 @@
>  #include "qemu/rcu.h"
>  #include "sysemu/hostmem.h"
>  #include "hw/cxl/cxl.h"
> +#include "hw/pci/msi.h"
> +#include "hw/pci/msix.h"
> +
> +#define DWORD_BYTE 4
> +
> +bool cxl_doe_compliance_rsp(DOECap *doe_cap)
> +{
> +CompRsp *rsp = (doe_cap->pdev)->cxl_cstate.compliance.response;
> +struct compliance_req_header *req = pcie_doe_get_write_mbox_ptr(doe_cap);
> +uint32_t type, req_len = 0, rsp_len = 0;
> +
> +type = req->req_code;
> +
> +switch (type) {
> +case CXL_COMP_MODE_CAP:
> +req_len = sizeof(struct cxl_compliance_cap_req);
> +rsp_len = sizeof(struct cxl_compliance_cap_rsp);
> +rsp->cap_rsp.status = 0x0;

Makes sense to set this for all messages we are replying to.
Also good to use a define to make it clear 0 is success.
Reference to table 276.

> +rsp->cap_rsp.available_cap_bitmask = 0;
> +rsp->cap_rsp.enabled_cap_bitmask = 0;

A reference for what goes in this bitmasks would be helpful if available.
0 doesn't make a lot of sense.  Whilst we are obviously only faking the
compliance interface it should be internally consistent.

> +break;
> +case CXL_COMP_MODE_STATUS:
> +req_len = sizeof(struct cxl_compliance_status_req);
> +rsp_len = sizeof(struct cxl_compliance_status_rsp);
> +rsp->status_rsp.cap_bitfield = 0;
> +rsp->status_rsp.cache_size = 0;
> +rsp->status_rsp.cache_size_units = 0;

Might be good to have some explanatory comments in here for the values.
I think these are from the description in table 258 but not totally sure.

> +break;
> +case CXL_COMP_MODE_HALT:
> +req_len = sizeof(struct cxl_compliance_halt_req);
> +rsp_len = sizeof(struct cxl_compliance_halt_rsp);
> +break;
> +case CXL_COMP_MODE_MULT_WR_STREAM:
> +req_len = sizeof(struct cxl_compliance_multi_write_streaming_req);
> +rsp_len = sizeof(struct cxl_compliance_multi_write_streaming_rsp);
> +break;
> +case CXL_COMP_MODE_PRO_CON:
> +req_len = sizeof(struct cxl_compliance_producer_consumer_req);
> +rsp_len = sizeof(struct cxl_compliance_producer_consumer_rsp);
> +break;
> +case CXL_COMP_MODE_BOGUS:
> +req_len = sizeof(struct cxl_compliance_bogus_writes_req);
> +rsp_len = sizeof(struct cxl_compliance_bogus_writes_rsp);
> +break;
> +case CXL_COMP_MODE_INJ_POISON:
> +req_len = sizeof(struct cxl_compliance_inject_poison_req);
> +rsp_len = sizeof(struct cxl_compliance_inject_poison_rsp);
> +break;
> +case CXL_COMP_MODE_INJ_CRC:
> +req_len = sizeof(struct cxl_compliance_inject_crc_req);
> +rsp_len = sizeof(struct cxl_compliance_inject_crc_rsp);
> +break;
> +case CXL_COMP_MODE_INJ_FC:
> +req_len = sizeof(struct cxl_compliance_inject_flow_ctrl_req);
> +rsp_len = sizeof(struct cxl_compliance_inject_flow_ctrl_rsp);
> +break;
> +case CXL_COMP_MODE_TOGGLE_CACHE:
> +req_len = sizeof(struct cxl_compliance_toggle_cache_flush_req);
> +rsp_len = sizeof(struct cxl_compliance_toggle_cache_flush_rsp);
> +break;
> +case CXL_COMP_MODE_INJ_MAC:
> +req_len = sizeof(struct cxl_compliance_inject_mac_delay_req);
> +rsp_len = sizeof(struct cxl_compliance_inject_mac_delay_rsp);
> +break;
> +case CXL_COMP_MODE_INS_UNEXP_MAC:
> +req_len = sizeof(struct cxl_compliance_insert_unexp_mac_req);
> +rsp_len = sizeof(struct cxl_compliance_insert_unexp_mac_rsp);
> +break;
> +case CXL_COMP_MODE_INJ_VIRAL:
> +req_len = 

[PATCH v6 cxl2.0-v6-doe 4/6] cxl/compliance: CXL Compliance Data Object Exchange implementation

2021-06-10 Thread Chris Browy
From: hchkuo 

The Data Object Exchange implementation of CXL Compliance Mode is
referring to "Compute Express Link (CXL) Specification, Rev. 2.0, Oct.
2020".

The data structure of CXL compliance request and response is added to
the header. Due to the scope limitation of QEMU, most of the compliance
response is limited to returning corresponding length.

A DOE capability of CXL Compliance is added to hw/mem/cxl_type3.c with
capability offset 0x160. The config read/write to this capability range
can be generated in the OS to request the Compliance info.

Signed-off-by: hchkuo 
Signed-off-by: Chris Browy 
---
 hw/mem/cxl_type3.c  | 147 
 include/hw/cxl/cxl_compliance.h | 293 
 include/hw/cxl/cxl_component.h  |   3 +
 include/hw/cxl/cxl_device.h |   3 +
 include/hw/cxl/cxl_pci.h|   1 +
 5 files changed, 447 insertions(+)
 create mode 100644 include/hw/cxl/cxl_compliance.h

diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index bf33ddb915..569872eb36 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -13,6 +13,134 @@
 #include "qemu/rcu.h"
 #include "sysemu/hostmem.h"
 #include "hw/cxl/cxl.h"
+#include "hw/pci/msi.h"
+#include "hw/pci/msix.h"
+
+#define DWORD_BYTE 4
+
+bool cxl_doe_compliance_rsp(DOECap *doe_cap)
+{
+CompRsp *rsp = (doe_cap->pdev)->cxl_cstate.compliance.response;
+struct compliance_req_header *req = pcie_doe_get_write_mbox_ptr(doe_cap);
+uint32_t type, req_len = 0, rsp_len = 0;
+
+type = req->req_code;
+
+switch (type) {
+case CXL_COMP_MODE_CAP:
+req_len = sizeof(struct cxl_compliance_cap_req);
+rsp_len = sizeof(struct cxl_compliance_cap_rsp);
+rsp->cap_rsp.status = 0x0;
+rsp->cap_rsp.available_cap_bitmask = 0;
+rsp->cap_rsp.enabled_cap_bitmask = 0;
+break;
+case CXL_COMP_MODE_STATUS:
+req_len = sizeof(struct cxl_compliance_status_req);
+rsp_len = sizeof(struct cxl_compliance_status_rsp);
+rsp->status_rsp.cap_bitfield = 0;
+rsp->status_rsp.cache_size = 0;
+rsp->status_rsp.cache_size_units = 0;
+break;
+case CXL_COMP_MODE_HALT:
+req_len = sizeof(struct cxl_compliance_halt_req);
+rsp_len = sizeof(struct cxl_compliance_halt_rsp);
+break;
+case CXL_COMP_MODE_MULT_WR_STREAM:
+req_len = sizeof(struct cxl_compliance_multi_write_streaming_req);
+rsp_len = sizeof(struct cxl_compliance_multi_write_streaming_rsp);
+break;
+case CXL_COMP_MODE_PRO_CON:
+req_len = sizeof(struct cxl_compliance_producer_consumer_req);
+rsp_len = sizeof(struct cxl_compliance_producer_consumer_rsp);
+break;
+case CXL_COMP_MODE_BOGUS:
+req_len = sizeof(struct cxl_compliance_bogus_writes_req);
+rsp_len = sizeof(struct cxl_compliance_bogus_writes_rsp);
+break;
+case CXL_COMP_MODE_INJ_POISON:
+req_len = sizeof(struct cxl_compliance_inject_poison_req);
+rsp_len = sizeof(struct cxl_compliance_inject_poison_rsp);
+break;
+case CXL_COMP_MODE_INJ_CRC:
+req_len = sizeof(struct cxl_compliance_inject_crc_req);
+rsp_len = sizeof(struct cxl_compliance_inject_crc_rsp);
+break;
+case CXL_COMP_MODE_INJ_FC:
+req_len = sizeof(struct cxl_compliance_inject_flow_ctrl_req);
+rsp_len = sizeof(struct cxl_compliance_inject_flow_ctrl_rsp);
+break;
+case CXL_COMP_MODE_TOGGLE_CACHE:
+req_len = sizeof(struct cxl_compliance_toggle_cache_flush_req);
+rsp_len = sizeof(struct cxl_compliance_toggle_cache_flush_rsp);
+break;
+case CXL_COMP_MODE_INJ_MAC:
+req_len = sizeof(struct cxl_compliance_inject_mac_delay_req);
+rsp_len = sizeof(struct cxl_compliance_inject_mac_delay_rsp);
+break;
+case CXL_COMP_MODE_INS_UNEXP_MAC:
+req_len = sizeof(struct cxl_compliance_insert_unexp_mac_req);
+rsp_len = sizeof(struct cxl_compliance_insert_unexp_mac_rsp);
+break;
+case CXL_COMP_MODE_INJ_VIRAL:
+req_len = sizeof(struct cxl_compliance_inject_viral_req);
+rsp_len = sizeof(struct cxl_compliance_inject_viral_rsp);
+break;
+case CXL_COMP_MODE_INJ_ALMP:
+req_len = sizeof(struct cxl_compliance_inject_almp_req);
+rsp_len = sizeof(struct cxl_compliance_inject_almp_rsp);
+break;
+case CXL_COMP_MODE_IGN_ALMP:
+req_len = sizeof(struct cxl_compliance_ignore_almp_req);
+rsp_len = sizeof(struct cxl_compliance_ignore_almp_rsp);
+break;
+case CXL_COMP_MODE_INJ_BIT_ERR:
+req_len = sizeof(struct cxl_compliance_inject_bit_err_in_flit_req);
+rsp_len = sizeof(struct cxl_compliance_inject_bit_err_in_flit_rsp);
+break;
+default:
+break;
+}
+
+/* Discard if request length mismatched */
+if (pcie_doe_get_obj_len(req) < DIV_ROUND_UP(req_len, DWORD_BYTE)) {
+return false;
+}
+