Re: [PATCH 1/1] Added support for get/set alert configuration commands

[Sweta Kumari <s5.kumari@samsung.com>]

[-- Attachment #1: Type: text/plain, Size: 8730 bytes --]

On 04/02/25 05:29PM, Fan Ni wrote:
>On Mon, Feb 03, 2025 at 05:14:45PM +0530, Sweta Kumari wrote:
>> Signed-off-by: Sweta Kumari <s5.kumari@samsung.com>
>> Reviewed-by: Alok Rathore <alok.rathore@samsung.com>
>> Reviewed-by: Krishna Kanth Reddy <krish.reddy@samsung.com>
>Hi Sweta,
>Since this series only have one patch, you do not need a cover letter.
>Move the commit comment in the first patch to this one and remove the
>first one.
>
>Fan
Hi Fan Ni,

Thank you for your feedback. I'll fix this in V2 patch.
>> ---
>>  hw/cxl/cxl-mailbox-utils.c  | 91 +++++++++++++++++++++++++++++++++++++
>>  hw/mem/cxl_type3.c          | 20 ++++++++
>>  include/hw/cxl/cxl_device.h | 24 ++++++++++
>>  3 files changed, 135 insertions(+)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index 9c7ea5bc35..7a054c059d 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -86,6 +86,9 @@ enum {
>>          #define GET_PARTITION_INFO     0x0
>>          #define GET_LSA       0x2
>>          #define SET_LSA       0x3
>> +    HEALTH_INFO_ALERTS = 0x42,
>> +        #define GET_ALERT_CONFIGURATION 0x1
>> +        #define SET_ALERT_CONFIGURATION 0x2
>>      SANITIZE    = 0x44,
>>          #define OVERWRITE     0x0
>>          #define SECURE_ERASE  0x1
>> @@ -1625,6 +1628,90 @@ static CXLRetCode cmd_ccls_set_lsa(const struct cxl_cmd *cmd,
>>      return CXL_MBOX_SUCCESS;
>>  }
>>
>> +/* CXL r3.1 Section 8.2.9.9.3.2 Get Alert Configuration (Opcode 4201h) */
>As mentioned by Jonathan, use r3.2.
Ok, I'll address this in V2 patch.
>> +static CXLRetCode cmd_get_alert_config(const struct cxl_cmd *cmd,
>> +                                   uint8_t *payload_in,
>> +                                   size_t len_in,
>> +                                   uint8_t *payload_out,
>> +                                   size_t *len_out,
>> +                                   CXLCCI *cci)
>The code indent here is not right.
>+static CXLRetCode cmd_get_alert_config(const struct cxl_cmd *cmd,
>+                                       uint8_t *payload_in,
>+                                       size_t len_in,
>+                                       uint8_t *payload_out,
>+                                       size_t *len_out,
>+                                       CXLCCI *cci)
>
Ok, will fix the alignment as suggested.
>> +{
>> +    CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
>> +    CXLAlertConfig *out = (void *)payload_out;
>> +    memcpy(out, &ct3d->alert_config, sizeof(ct3d->alert_config));
>> +    *len_out = sizeof(ct3d->alert_config);
>> +    return CXL_MBOX_SUCCESS;
>> +
>> +}
>> +
>> +/* CXL r3.1 Section 8.2.9.9.3.3 Set Alert Configuration (Opcode 4202h) */
>> +static CXLRetCode cmd_set_alert_config(const struct cxl_cmd *cmd,
>> +                                   uint8_t *payload_in,
>> +                                   size_t len_in,
>> +                                   uint8_t *payload_out,
>> +                                   size_t *len_out,
>> +                                   CXLCCI *cci)
>indent issue.
>
>Fan
Ok, I'll fix this in V2 patch.
>> +{
>> +    CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
>> +    CXLAlertConfig *alert_config = &ct3d->alert_config;
>> +    struct {
>> +        CXLValidEnableAlerts valid_alert_actions;
>> +        CXLValidEnableAlerts enable_alert_actions;
>> +        uint8_t lupwt;
>> +        uint8_t reserve;
>> +        uint16_t dotpwt;
>> +        uint16_t dutpwt;
>> +        uint16_t cvmepwt;
>> +        uint16_t cpmepwt;
>> +    } QEMU_PACKED *in = (void *)payload_in;
>> +
>> +    if (in->valid_alert_actions.lupwt) {
>> +        if ((in->lupwt > 100) || (in->lupwt >= alert_config->lucat)) {
>> +            return CXL_MBOX_INVALID_INPUT;
>> +        }
>> +
>> +        if (in->enable_alert_actions.lupwt) {
>> +            alert_config->lupwt = in->lupwt;
>> +        }
>> +        alert_config->enable_alerts.lupwt = in->enable_alert_actions.lupwt;
>> +    }
>> +
>> +    if (in->valid_alert_actions.dotpwt) {
>> +        if (in->dotpwt >= alert_config->dotcat) {
>> +            return CXL_MBOX_INVALID_INPUT;
>> +        }
>> +        if (in->enable_alert_actions.dotpwt) {
>> +            alert_config->dotpwt = in->dotpwt;
>> +        }
>> +        alert_config->enable_alerts.dotpwt = in->enable_alert_actions.dotpwt;
>> +    }
>> +
>> +    if (in->valid_alert_actions.dutpwt) {
>> +        if (in->dutpwt < alert_config->dutcat) {
>> +            return CXL_MBOX_INVALID_INPUT;
>> +        }
>> +        if (in->enable_alert_actions.dutpwt) {
>> +            alert_config->dutpwt = in->dutpwt;
>> +        }
>> +        alert_config->enable_alerts.dutpwt = in->enable_alert_actions.dutpwt;
>> +    }
>> +
>> +    if (in->valid_alert_actions.cvmepwt) {
>> +        if (in->enable_alert_actions.cvmepwt) {
>> +            alert_config->cvmepwt = in->cvmepwt;
>> +        }
>> +        alert_config->enable_alerts.cvmepwt = in->valid_alert_actions.cvmepwt;
>> +    }
>> +
>> +    if (in->valid_alert_actions.cpmepwt) {
>> +        if (in->enable_alert_actions.cpmepwt) {
>> +            alert_config->cpmepwt = in->cpmepwt;
>> +        }
>> +        alert_config->enable_alerts.cpmepwt = in->valid_alert_actions.cpmepwt;
>> +    }
>> +    return CXL_MBOX_SUCCESS;
>> +}
>> +
>>  /* Perform the actual device zeroing */
>>  static void __do_sanitization(CXLType3Dev *ct3d)
>>  {
>> @@ -2859,6 +2946,10 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
>>      [CCLS][GET_LSA] = { "CCLS_GET_LSA", cmd_ccls_get_lsa, 8, 0 },
>>      [CCLS][SET_LSA] = { "CCLS_SET_LSA", cmd_ccls_set_lsa,
>>          ~0, CXL_MBOX_IMMEDIATE_CONFIG_CHANGE | CXL_MBOX_IMMEDIATE_DATA_CHANGE },
>> +    [HEALTH_INFO_ALERTS][GET_ALERT_CONFIGURATION] = {"GET_ALERT_CONFIGURATION",
>> +        cmd_get_alert_config, 0, 0 },
>> +    [HEALTH_INFO_ALERTS][SET_ALERT_CONFIGURATION] = {"SET_ALERT_CONFIGURATION",
>> +        cmd_set_alert_config, 12, CXL_MBOX_IMMEDIATE_POLICY_CHANGE },
>>      [SANITIZE][OVERWRITE] = { "SANITIZE_OVERWRITE", cmd_sanitize_overwrite, 0,
>>          (CXL_MBOX_IMMEDIATE_DATA_CHANGE |
>>           CXL_MBOX_SECURITY_STATE_CHANGE |
>> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
>> index 5f365afb4d..ce5a4962da 100644
>> --- a/hw/mem/cxl_type3.c
>> +++ b/hw/mem/cxl_type3.c
>> @@ -956,6 +956,25 @@ static DOEProtocol doe_comp_prot[] = {
>>      { }
>>  };
>>
>> +/*
>> + * Initialize CXL device alerts with default threshold values.
>> + */
>> +static void init_alert_config(CXLType3Dev *ct3d)
>> +{
>> +    CXLAlertConfig *alert_config = &ct3d->alert_config;
>> +
>> +    memset(&alert_config->valid_alerts, 0, sizeof(CXLValidEnableAlerts));
>> +    memset(&alert_config->enable_alerts, 0, sizeof(CXLValidEnableAlerts));
>> +    alert_config->lucat = 75;
>> +    alert_config->lupwt = 0;
>> +    alert_config->dotcat = 35;
>> +    alert_config->dutcat = 10;
>> +    alert_config->dotpwt = 25;
>> +    alert_config->dutpwt = 20;
>> +    alert_config->cvmepwt = 0;
>> +    alert_config->cpmepwt = 0;
>> +}
>> +
>>  void ct3_realize(PCIDevice *pci_dev, Error **errp)
>>  {
>>      ERRP_GUARD();
>> @@ -1030,6 +1049,7 @@ void ct3_realize(PCIDevice *pci_dev, Error **errp)
>>          goto err_free_special_ops;
>>      }
>>
>> +    init_alert_config(ct3d);
>>      pcie_cap_deverr_init(pci_dev);
>>      /* Leave a bit of room for expansion */
>>      rc = pcie_aer_init(pci_dev, PCI_ERR_VER, 0x200, PCI_ERR_SIZEOF, NULL);
>> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
>> index a64739be25..6acae7d74d 100644
>> --- a/include/hw/cxl/cxl_device.h
>> +++ b/include/hw/cxl/cxl_device.h
>> @@ -581,6 +581,28 @@ typedef struct CXLSetFeatureInfo {
>>      size_t data_size;
>>  } CXLSetFeatureInfo;
>>
>> +typedef struct CXLValidEnableAlerts {
>> +    uint8_t lupwt:1;
>> +    uint8_t dotpwt:1;
>> +    uint8_t dutpwt:1;
>> +    uint8_t cvmepwt:1;
>> +    uint8_t cpmepwt:1;
>> +    uint8_t reserved:3;
>> +} CXLValidEnableAlerts;
>> +
>> +typedef struct CXLAlertConfig {
>> +    CXLValidEnableAlerts valid_alerts;
>> +    CXLValidEnableAlerts enable_alerts;
>> +    uint8_t lucat;
>> +    uint8_t lupwt;
>> +    uint16_t dotcat;
>> +    uint16_t dutcat;
>> +    uint16_t dotpwt;
>> +    uint16_t dutpwt;
>> +    uint16_t cvmepwt;
>> +    uint16_t cpmepwt;
>> +} QEMU_PACKED CXLAlertConfig;
>> +
>>  struct CXLType3Dev {
>>      /* Private */
>>      PCIDevice parent_obj;
>> @@ -605,6 +627,8 @@ struct CXLType3Dev {
>>      CXLCCI vdm_fm_owned_ld_mctp_cci;
>>      CXLCCI ld0_cci;
>>
>> +    CXLAlertConfig alert_config;
>> +
>>      /* PCIe link characteristics */
>>      PCIExpLinkSpeed speed;
>>      PCIExpLinkWidth width;
>> --
>> 2.34.1
>>
>
>-- 
>Fan Ni (From gmail)

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]