* [PATCH 0/1] CXL CCI Health Information & Alerts Commands implementation [not found] <CGME20250203114457epcas5p1569ad2154f5ca08145e74c44efee1e1e@epcas5p1.samsung.com> @ 2025-02-03 11:44 ` Sweta Kumari [not found] ` <CGME20250203114459epcas5p33839b8e8150427947f9b5419ce9cd611@epcas5p3.samsung.com> 0 siblings, 1 reply; 6+ messages in thread From: Sweta Kumari @ 2025-02-03 11:44 UTC (permalink / raw) To: qemu-devel Cc: gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore, Sweta Kumari CXL CCI get/set alert config commands implmented as per CXL Specification 3.1 8.2.9.9.3 1)get alert configuration(Opcode 4201h) 2)set alert configuration(Opcode 4202h) The patches are generated against the Johnathan's tree https://gitlab.com/jic23/qemu.git and branch cxl-2024-11-27. 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> Sweta Kumari (1): Added support for get/set alert configuration commands 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(+) -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <CGME20250203114459epcas5p33839b8e8150427947f9b5419ce9cd611@epcas5p3.samsung.com>]
* [PATCH 1/1] Added support for get/set alert configuration commands [not found] ` <CGME20250203114459epcas5p33839b8e8150427947f9b5419ce9cd611@epcas5p3.samsung.com> @ 2025-02-03 11:44 ` Sweta Kumari 2025-02-04 11:15 ` Jonathan Cameron via 2025-02-04 17:29 ` Fan Ni 0 siblings, 2 replies; 6+ messages in thread From: Sweta Kumari @ 2025-02-03 11:44 UTC (permalink / raw) To: qemu-devel Cc: gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore, Sweta Kumari 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> --- 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) */ +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) +{ + 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) +{ + 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 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Added support for get/set alert configuration commands 2025-02-03 11:44 ` [PATCH 1/1] Added support for get/set alert configuration commands Sweta Kumari @ 2025-02-04 11:15 ` Jonathan Cameron via 2025-02-11 11:07 ` Sweta Kumari 2025-02-04 17:29 ` Fan Ni 1 sibling, 1 reply; 6+ messages in thread From: Jonathan Cameron via @ 2025-02-04 11:15 UTC (permalink / raw) To: Sweta Kumari Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore On Mon, 3 Feb 2025 17:14:45 +0530 Sweta Kumari <s5.kumari@samsung.com> 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, Thanks for the patch. Great to see someone working on these features. As in previous thread, in general RB tags that are given before public posting aren't really very useful. Reviews can mean very different things in different companies / groups within them. Much prefer to see a final round of review done on the public mailing list if possible. Various comments inline. Jonathan > --- > 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) */ For new commands, preference is for spec references being to the latest spec as that's the one that is downloadable via a click through on the website. > +static CXLRetCode cmd_get_alert_config(const struct cxl_cmd *cmd, > + uint8_t *payload_in, Align immediate after ( unless we have very long lines. > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out, > + CXLCCI *cci) > +{ > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > + CXLAlertConfig *out = (void *)payload_out; blank line here. > + memcpy(out, &ct3d->alert_config, sizeof(ct3d->alert_config)); > + *len_out = sizeof(ct3d->alert_config); blank line here. > + 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) > +{ > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > + CXLAlertConfig *alert_config = &ct3d->alert_config; > + struct { > + CXLValidEnableAlerts valid_alert_actions; > + CXLValidEnableAlerts enable_alert_actions; uint8_t for both of these + defines. > + uint8_t lupwt; Longer names here as below. > + 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)) { Add a spec reference for these constraints. Looks like they are in the get alert configuration output table. > + return CXL_MBOX_INVALID_INPUT; > + } > + > + if (in->enable_alert_actions.lupwt) { > + alert_config->lupwt = in->lupwt; That's interesting. Do we only write it when enabled. I would have thought valid was enough. Spec reference in a comment would help for this first case at least. No need to replicate in the others if it is a common thing. > + } > + 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; Easier to read as ct3d_alert_config = (CXLAlertConfig) { .life_used_critical_thresh = 75, ... You can skip anything where a default of 0 is an obvious default. Perhaps the valid_alerts and enable alerts for instance. }; > +} > + > 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; Give these longer names. life_used_warning_threshold etc etc as we don't want to go find the spec to figure out what they are. > + uint8_t dotpwt:1; > + uint8_t dutpwt:1; > + uint8_t cvmepwt:1; > + uint8_t cpmepwt:1; > + uint8_t reserved:3; Generally bit fields do not play well with potential endian variations or low level code in general. There is too much flexibility in how they are stored. So better to use a uint8_t and some defines for the bits that are set. > +} CXLValidEnableAlerts; > + > +typedef struct CXLAlertConfig { > + CXLValidEnableAlerts valid_alerts; > + CXLValidEnableAlerts enable_alerts; > + uint8_t lucat; Longer names. These want to be at least vaguely comprehensible without the spec in front of us. lif_used_critical_thresh life_used_warning_thresh (I'm not sure why programmable matters!) etc > + 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; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Added support for get/set alert configuration commands 2025-02-04 11:15 ` Jonathan Cameron via @ 2025-02-11 11:07 ` Sweta Kumari 0 siblings, 0 replies; 6+ messages in thread From: Sweta Kumari @ 2025-02-11 11:07 UTC (permalink / raw) To: Jonathan Cameron Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore [-- Attachment #1: Type: text/plain, Size: 10545 bytes --] On 04/02/25 11:15AM, Jonathan Cameron wrote: >On Mon, 3 Feb 2025 17:14:45 +0530 >Sweta Kumari <s5.kumari@samsung.com> 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, > >Thanks for the patch. Great to see someone working on these >features. As in previous thread, in general RB tags that are >given before public posting aren't really very useful. Reviews >can mean very different things in different companies / groups within >them. Much prefer to see a final round of review done on the public >mailing list if possible. > >Various comments inline. > >Jonathan > Hi Jonathan, Thank you for the 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) */ > >For new commands, preference is for spec references being to >the latest spec as that's the one that is downloadable via a click through >on the website. > Ok, Will fix it in V2 patch. >> +static CXLRetCode cmd_get_alert_config(const struct cxl_cmd *cmd, >> + uint8_t *payload_in, > >Align immediate after ( unless we have very long lines. > >> + size_t len_in, >> + uint8_t *payload_out, >> + size_t *len_out, >> + CXLCCI *cci) >> +{ >> + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); >> + CXLAlertConfig *out = (void *)payload_out; > >blank line here. > Ok. >> + memcpy(out, &ct3d->alert_config, sizeof(ct3d->alert_config)); >> + *len_out = sizeof(ct3d->alert_config); > >blank line here. > Ok. >> + 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) >> +{ >> + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); >> + CXLAlertConfig *alert_config = &ct3d->alert_config; >> + struct { >> + CXLValidEnableAlerts valid_alert_actions; >> + CXLValidEnableAlerts enable_alert_actions; > >uint8_t for both of these + defines. > >> + uint8_t lupwt; > >Longer names here as below. > Ok, I'll update both with defines and longer names as suggested. >> + 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)) { > >Add a spec reference for these constraints. Looks like they >are in the get alert configuration output table. > Ok, as you mentioned, I'll remove the if (in->lupwt > 100) check and retain only the if (in->lupwt >= alert_config->lucat) check. As per CXL spec 3.2. The life used warning threshold shall be less than the life used critical alert value. >> + return CXL_MBOX_INVALID_INPUT; >> + } >> + >> + if (in->enable_alert_actions.lupwt) { >> + alert_config->lupwt = in->lupwt; > >That's interesting. Do we only write it when enabled. I would have >thought valid was enough. Spec reference in a comment would help >for this first case at least. No need to replicate in the >others if it is a common thing. > > Based on your feedback, I'll remove the checks for enable alert actions and rely only on valid alert actions to determine if the field should be processed, as it is sufficient for this operation >> + } >> + 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; >Easier to read as > ct3d_alert_config = (CXLAlertConfig) { > .life_used_critical_thresh = 75, >... > >You can skip anything where a default of 0 is an obvious default. >Perhaps the valid_alerts and enable alerts for instance. > Ok, I'll address this in V2 patch. > }; >> +} >> + >> 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; > >Give these longer names. >life_used_warning_threshold etc >etc as we don't want to go find the spec to figure >out what they are. > > Ok, I'll update this in V2 patch. >> + uint8_t dotpwt:1; >> + uint8_t dutpwt:1; >> + uint8_t cvmepwt:1; >> + uint8_t cpmepwt:1; >> + uint8_t reserved:3; > >Generally bit fields do not play well with potential endian >variations or low level code in general. There is too much >flexibility in how they are stored. So better to use a >uint8_t and some defines for the bits that are set. > > Ok. >> +} CXLValidEnableAlerts; >> + >> +typedef struct CXLAlertConfig { >> + CXLValidEnableAlerts valid_alerts; >> + CXLValidEnableAlerts enable_alerts; >> + uint8_t lucat; >Longer names. These want to be at least vaguely comprehensible >without the spec in front of us. >lif_used_critical_thresh >life_used_warning_thresh (I'm not sure why programmable matters!) >etc > Ok, will fix this in V2 patch. >> + 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; > [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Added support for get/set alert configuration commands 2025-02-03 11:44 ` [PATCH 1/1] Added support for get/set alert configuration commands Sweta Kumari 2025-02-04 11:15 ` Jonathan Cameron via @ 2025-02-04 17:29 ` Fan Ni 2025-02-11 11:34 ` Sweta Kumari 1 sibling, 1 reply; 6+ messages in thread From: Fan Ni @ 2025-02-04 17:29 UTC (permalink / raw) To: Sweta Kumari Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore 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 > --- > 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. > +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) > +{ > + 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 > +{ > + 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) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] Added support for get/set alert configuration commands 2025-02-04 17:29 ` Fan Ni @ 2025-02-11 11:34 ` Sweta Kumari 0 siblings, 0 replies; 6+ messages in thread From: Sweta Kumari @ 2025-02-11 11:34 UTC (permalink / raw) To: Fan Ni Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore [-- 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 --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-02-11 13:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20250203114457epcas5p1569ad2154f5ca08145e74c44efee1e1e@epcas5p1.samsung.com>
2025-02-03 11:44 ` [PATCH 0/1] CXL CCI Health Information & Alerts Commands implementation Sweta Kumari
[not found] ` <CGME20250203114459epcas5p33839b8e8150427947f9b5419ce9cd611@epcas5p3.samsung.com>
2025-02-03 11:44 ` [PATCH 1/1] Added support for get/set alert configuration commands Sweta Kumari
2025-02-04 11:15 ` Jonathan Cameron via
2025-02-11 11:07 ` Sweta Kumari
2025-02-04 17:29 ` Fan Ni
2025-02-11 11:34 ` Sweta Kumari
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).