* Re: [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h)
2025-05-22 6:31 ` [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h) Vinayak Holikatti
@ 2025-05-22 16:48 ` Fan Ni
2025-07-01 12:59 ` Alok Rathore
2025-05-22 19:15 ` Davidlohr Bueso
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Fan Ni @ 2025-05-22 16:48 UTC (permalink / raw)
To: Vinayak Holikatti
Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g,
krish.reddy, a.manzanares, alok.rathore
On Thu, May 22, 2025 at 12:01:35PM +0530, Vinayak Holikatti wrote:
> CXL spec 3.2 section 7.6.7.5.2 describes Get Head Info.
>
> Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com>
> ---
> This patch is generated against Jonathan Cameron's branch cxl-2025-03-20
>
> hw/cxl/cxl-mailbox-utils.c | 21 +++++++++
> hw/cxl/mhsld/mhsld.c | 92 ++++++++++++++++++++++++++++++++++++-
> hw/cxl/mhsld/mhsld.h | 26 +++++++++++
> include/hw/cxl/cxl_device.h | 6 +++
> 4 files changed, 144 insertions(+), 1 deletion(-)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index a02d130926..4f25caecea 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -122,6 +122,7 @@ enum {
> #define MANAGEMENT_COMMAND 0x0
> MHD = 0x55,
> #define GET_MHD_INFO 0x0
> + #define GET_MHD_HEAD_INFO 0x1
> };
>
> /* CCI Message Format CXL r3.1 Figure 7-19 */
> @@ -267,6 +268,23 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
> return CXL_MBOX_UNSUPPORTED;
> }
>
> +/*
> + * CXL r3.2 section 7.6.7.5.2 - Get Multi-Headed Head Info (Opcode 5501h)
That does not match the section subject of the spec
Get Head Info (...
> + */
> +static CXLRetCode cmd_mhd_get_head_info(const struct cxl_cmd *cmd,
> + uint8_t *payload_in, size_t len_in,
> + uint8_t *payload_out, size_t *len_out,
> + CXLCCI *cci)
The indent here is not right.
> +static CXLRetCode cmd_mhd_get_head_info(const struct cxl_cmd *cmd,
> + uint8_t *payload_in, size_t len_in,
> +{
> + CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
> + CXLType3Class *cvc = CXL_TYPE3_GET_CLASS(ct3d);
> + if (cvc->mhd_get_head_info) {
> + return cvc->mhd_get_head_info(cmd, payload_in, len_in, payload_out,
> + len_out, cci);
indent needs fix.
> + }
> + return CXL_MBOX_UNSUPPORTED;
> +}
> +
> static CXLRetCode cmd_events_get_records(const struct cxl_cmd *cmd,
> uint8_t *payload_in, size_t len_in,
> uint8_t *payload_out, size_t *len_out,
> @@ -3429,6 +3447,7 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> "MEDIA_AND_POISON_GET_SCAN_MEDIA_RESULTS",
> cmd_media_get_scan_media_results, 0, 0 },
> [MHD][GET_MHD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_info, 2, 0},
> + [MHD][GET_MHD_HEAD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_head_info, 2, 0},
> };
>
> static const struct cxl_cmd cxl_cmd_set_dcd[256][256] = {
> @@ -3761,6 +3780,8 @@ static const struct cxl_cmd cxl_cmd_set_t3_fm_owned_ld_mctp[256][256] = {
> [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
> [TUNNEL][MANAGEMENT_COMMAND] = { "TUNNEL_MANAGEMENT_COMMAND",
> cmd_tunnel_management_cmd, ~0, 0 },
> + [MHD][GET_MHD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_info, 2, 0},
> + [MHD][GET_MHD_HEAD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_head_info, 2, 0},
> };
>
> void cxl_initialize_t3_fm_owned_ld_mctpcci(CXLCCI *cci, DeviceState *d,
> diff --git a/hw/cxl/mhsld/mhsld.c b/hw/cxl/mhsld/mhsld.c
> index 9f633b3bed..981546b5ff 100644
> --- a/hw/cxl/mhsld/mhsld.c
> +++ b/hw/cxl/mhsld/mhsld.c
> @@ -61,9 +61,57 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
> +/*
> + * CXL r3.2 section 7.6.7.5.2 - Get Head Info (Opcode 5501h)
> + *
> + * This command retrieves the number of heads, number of supported LDs,
> + * and Head-to-LD mapping of a Multi-Headed device.
> + */
> +static CXLRetCode cmd_mhd_get_head_info(const struct cxl_cmd *cmd,
> + uint8_t *payload_in, size_t len_in,
> + uint8_t *payload_out, size_t *len_out,
> + CXLCCI *cci)
> +{
> + CXLMHSLDState *s = CXL_MHSLD(cci->d);
> + MHDGetHeadInfoInput *input = (void *)payload_in;
> + MHDGetHeadInfoOutput *output = (void *)payload_out;
> + int i = 0;
> +
> + if (input->start_head > MHSLD_HEADS) {
Should be ">=" ?
Also, per the spec, we need also check the "number of heads" field, if
start_head and number of heads together point to a non-exist head, we
should return invalid input.
> + return CXL_MBOX_INVALID_INPUT;
> + }
> +
> + output->nr_heads = MIN((MHSLD_HEADS - input->start_head), input->nr_heads);
> + for (i = input->start_head; i < input->start_head + output->nr_heads; i++) {
> + output->head_info_list[i].port_number =
> + s->mhd_state->head_info_blocks[i].port_number;
> + output->head_info_list[i].max_link_width =
> + s->mhd_state->head_info_blocks[i].max_link_width;
> + output->head_info_list[i].nego_link_width =
> + s->mhd_state->head_info_blocks[i].nego_link_width;
> + output->head_info_list[i].supp_link_speeds_vector =
> + s->mhd_state->head_info_blocks[i].supp_link_speeds_vector;
> + output->head_info_list[i].max_link_speed =
> + s->mhd_state->head_info_blocks[i].max_link_speed;
> + output->head_info_list[i].current_link_speed =
> + s->mhd_state->head_info_blocks[i].current_link_speed;
> + output->head_info_list[i].ltssm_state =
> + s->mhd_state->head_info_blocks[i].ltssm_state;
> + output->head_info_list[i].first_nego_lane_num =
> + s->mhd_state->head_info_blocks[i].first_nego_lane_num;
> + output->head_info_list[i].link_state_flags =
> + s->mhd_state->head_info_blocks[i].link_state_flags;
> + }
> +
> + *len_out = sizeof(*output) + output->nr_heads * sizeof(MHDHeadInfoBlock);
> + return CXL_MBOX_SUCCESS;
> +}
> +
> static const struct cxl_cmd cxl_cmd_set_mhsld[256][256] = {
> [MHSLD_MHD][GET_MHD_INFO] = {"GET_MULTI_HEADED_INFO",
> cmd_mhd_get_info, 2, 0},
> + [MHSLD_MHD][GET_MHD_HEAD_INFO] = {"GET_HEAD_INFO",
> + cmd_mhd_get_head_info, 2, 0},
> };
>
> static const Property cxl_mhsld_props[] = {
> @@ -166,6 +214,47 @@ static void cxl_mhsld_state_initialize(CXLMHSLDState *s, size_t dc_size)
> s->mhd_state->nr_blocks = dc_size / MHSLD_BLOCK_SZ;
> }
>
> +
> +static void cxl_mhsld_init_head_info(CXLMHSLDState *s, PCIDevice *pdev)
> +{
> + uint16_t lnksta = 0;
> + uint16_t current_link_speed = 0;
> + uint16_t negotiated_link_width = 0;
> + uint16_t lnkcap = 0, lnkcap2 = 0;
> + uint16_t max_link_width = 0;
> + uint16_t max_link_speed = 0;
> + uint16_t supported_link_speeds_vector = 0;
> +
> + lnksta = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKSTA,
> + sizeof(lnksta));
> + lnkcap = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKCAP,
> + sizeof(lnkcap));
> + lnkcap2 = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKCAP2,
> + sizeof(lnkcap2));
> + supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
> + max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
> + max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
> + current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
> + negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
> +
> + s->mhd_state->head_info_blocks[s->mhd_head].port_number = s->mhd_head;
> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_width = max_link_width;
> + s->mhd_state->head_info_blocks[s->mhd_head].nego_link_width =
> + negotiated_link_width;
> + s->mhd_state->head_info_blocks[s->mhd_head].supp_link_speeds_vector =
> + supported_link_speeds_vector;
> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_speed =
> + max_link_speed;
> + s->mhd_state->head_info_blocks[s->mhd_head].current_link_speed =
> + current_link_speed;
> + s->mhd_state->head_info_blocks[s->mhd_head].ltssm_state = 0x7;
> + s->mhd_state->head_info_blocks[s->mhd_head].first_nego_lane_num = 0;
> + s->mhd_state->head_info_blocks[s->mhd_head].link_state_flags = 0;
> +}
> +
> /* Returns starting index of region in MHD map. */
> static inline size_t cxl_mhsld_find_dc_region_start(PCIDevice *d,
> CXLDCRegion *r)
> @@ -376,7 +465,7 @@ static void cxl_mhsld_realize(PCIDevice *pci_dev, Error **errp)
> }
>
> cxl_mhsld_state_initialize(s, dc_size);
> -
> + cxl_mhsld_init_head_info(s, pci_dev);
> /* Set the LD ownership for this head to this system */
> s->mhd_state->ldmap[s->mhd_head] = s->mhd_head;
> return;
> @@ -428,6 +517,7 @@ static void cxl_mhsld_class_init(ObjectClass *klass, void *data)
>
> CXLType3Class *cvc = CXL_TYPE3_CLASS(klass);
> cvc->mhd_get_info = cmd_mhd_get_info;
> + cvc->mhd_get_head_info = cmd_mhd_get_head_info;
> cvc->mhd_access_valid = cxl_mhsld_access_valid;
> cvc->mhd_reserve_extents = cxl_mhsld_reserve_extents;
> cvc->mhd_reclaim_extents = cxl_mhsld_reclaim_extents;
> diff --git a/hw/cxl/mhsld/mhsld.h b/hw/cxl/mhsld/mhsld.h
> index e7ead1f0d2..c9fbec71ca 100644
> --- a/hw/cxl/mhsld/mhsld.h
> +++ b/hw/cxl/mhsld/mhsld.h
> @@ -23,6 +23,18 @@
> */
> #define MHSLD_HEADS (8)
>
> +typedef struct MHDHeadInfoBlock {
> + uint8_t port_number;
> + uint8_t max_link_width;
> + uint8_t nego_link_width;
> + uint8_t supp_link_speeds_vector;
> + uint8_t max_link_speed;
> + uint8_t current_link_speed;
> + uint8_t ltssm_state;
> + uint8_t first_nego_lane_num;
> + uint8_t link_state_flags;
> +} QEMU_PACKED MHDHeadInfoBlock;
> +
> /*
> * The shared state cannot have 2 variable sized regions
> * so we have to max out the ldmap.
> @@ -32,6 +44,7 @@ typedef struct MHSLDSharedState {
> uint8_t nr_lds;
> uint8_t ldmap[MHSLD_HEADS];
> uint64_t nr_blocks;
> + MHDHeadInfoBlock head_info_blocks[MHSLD_HEADS];
> uint8_t blocks[];
> } MHSLDSharedState;
>
> @@ -52,6 +65,7 @@ struct CXLMHSLDClass {
> enum {
> MHSLD_MHD = 0x55,
> #define GET_MHD_INFO 0x0
> + #define GET_MHD_HEAD_INFO 0x1
> };
>
> /*
> @@ -72,4 +86,16 @@ typedef struct MHDGetInfoOutput {
> uint16_t resv2;
> uint8_t ldmap[];
> } QEMU_PACKED MHDGetInfoOutput;
> +
> +typedef struct MHDGetHeadInfoInput {
> + uint8_t start_head;
> + uint8_t nr_heads;
> +} QEMU_PACKED MHDGetHeadInfoInput;
> +
> +typedef struct MHDGetHeadInfoOutput {
> + uint8_t nr_heads;
> + uint8_t rsvd[3];
> + MHDHeadInfoBlock head_info_list[];
s/head_info_list/head_info/
Fan
> +} QEMU_PACKED MHDGetHeadInfoOutput;
> +
> #endif
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index ca515cab13..c93c71c45d 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -732,6 +732,12 @@ struct CXLType3Class {
> uint8_t *payload_out,
> size_t *len_out,
> CXLCCI *cci);
> + CXLRetCode (*mhd_get_head_info)(const struct cxl_cmd *cmd,
> + uint8_t *payload_in,
> + size_t len_in,
> + uint8_t *payload_out,
> + size_t *len_out,
> + CXLCCI *cci);
> bool (*mhd_access_valid)(PCIDevice *d, uint64_t addr, unsigned int size);
> bool (*mhd_reserve_extents)(PCIDevice *d,
> CxlDynamicCapacityExtentList *records,
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h)
2025-05-22 16:48 ` Fan Ni
@ 2025-07-01 12:59 ` Alok Rathore
0 siblings, 0 replies; 10+ messages in thread
From: Alok Rathore @ 2025-07-01 12:59 UTC (permalink / raw)
To: Fan Ni
Cc: qemu-devel, gost.dev, linux-cxl, dave, vishak.g, krish.reddy,
a.manzanares
[-- Attachment #1: Type: text/plain, Size: 12787 bytes --]
On 22/05/25 09:48AM, Fan Ni wrote:
>On Thu, May 22, 2025 at 12:01:35PM +0530, Vinayak Holikatti wrote:
>> CXL spec 3.2 section 7.6.7.5.2 describes Get Head Info.
>>
>> Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com>
>> ---
>> This patch is generated against Jonathan Cameron's branch cxl-2025-03-20
>>
>> hw/cxl/cxl-mailbox-utils.c | 21 +++++++++
>> hw/cxl/mhsld/mhsld.c | 92 ++++++++++++++++++++++++++++++++++++-
>> hw/cxl/mhsld/mhsld.h | 26 +++++++++++
>> include/hw/cxl/cxl_device.h | 6 +++
>> 4 files changed, 144 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index a02d130926..4f25caecea 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -122,6 +122,7 @@ enum {
>> #define MANAGEMENT_COMMAND 0x0
>> MHD = 0x55,
>> #define GET_MHD_INFO 0x0
>> + #define GET_MHD_HEAD_INFO 0x1
>> };
>>
>> /* CCI Message Format CXL r3.1 Figure 7-19 */
>> @@ -267,6 +268,23 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
>> return CXL_MBOX_UNSUPPORTED;
>> }
>>
>> +/*
>> + * CXL r3.2 section 7.6.7.5.2 - Get Multi-Headed Head Info (Opcode 5501h)
>That does not match the section subject of the spec
>Get Head Info (...
Thanks Fan Ni. Will fix in V1 patch.
>> + */
>> +static CXLRetCode cmd_mhd_get_head_info(const struct cxl_cmd *cmd,
>> + uint8_t *payload_in, size_t len_in,
>> + uint8_t *payload_out, size_t *len_out,
>> + CXLCCI *cci)
>The indent here is not right.
Okay. Will fix in V1 patch.
>> +static CXLRetCode cmd_mhd_get_head_info(const struct cxl_cmd *cmd,
>> + uint8_t *payload_in, size_t len_in,
>
>> +{
>> + CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
>> + CXLType3Class *cvc = CXL_TYPE3_GET_CLASS(ct3d);
>> + if (cvc->mhd_get_head_info) {
>> + return cvc->mhd_get_head_info(cmd, payload_in, len_in, payload_out,
>> + len_out, cci);
>
>indent needs fix.
Okay. Will fix in V1 patch.
>
>> + }
>> + return CXL_MBOX_UNSUPPORTED;
>> +}
>> +
>> static CXLRetCode cmd_events_get_records(const struct cxl_cmd *cmd,
>> uint8_t *payload_in, size_t len_in,
>> uint8_t *payload_out, size_t *len_out,
>> @@ -3429,6 +3447,7 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
>> "MEDIA_AND_POISON_GET_SCAN_MEDIA_RESULTS",
>> cmd_media_get_scan_media_results, 0, 0 },
>> [MHD][GET_MHD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_info, 2, 0},
>> + [MHD][GET_MHD_HEAD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_head_info, 2, 0},
>> };
>>
>> static const struct cxl_cmd cxl_cmd_set_dcd[256][256] = {
>> @@ -3761,6 +3780,8 @@ static const struct cxl_cmd cxl_cmd_set_t3_fm_owned_ld_mctp[256][256] = {
>> [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
>> [TUNNEL][MANAGEMENT_COMMAND] = { "TUNNEL_MANAGEMENT_COMMAND",
>> cmd_tunnel_management_cmd, ~0, 0 },
>> + [MHD][GET_MHD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_info, 2, 0},
>> + [MHD][GET_MHD_HEAD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_head_info, 2, 0},
>> };
>>
>> void cxl_initialize_t3_fm_owned_ld_mctpcci(CXLCCI *cci, DeviceState *d,
>> diff --git a/hw/cxl/mhsld/mhsld.c b/hw/cxl/mhsld/mhsld.c
>> index 9f633b3bed..981546b5ff 100644
>> --- a/hw/cxl/mhsld/mhsld.c
>> +++ b/hw/cxl/mhsld/mhsld.c
>> @@ -61,9 +61,57 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
>> return CXL_MBOX_SUCCESS;
>> }
>>
>> +/*
>> + * CXL r3.2 section 7.6.7.5.2 - Get Head Info (Opcode 5501h)
>> + *
>> + * This command retrieves the number of heads, number of supported LDs,
>> + * and Head-to-LD mapping of a Multi-Headed device.
>> + */
>> +static CXLRetCode cmd_mhd_get_head_info(const struct cxl_cmd *cmd,
>> + uint8_t *payload_in, size_t len_in,
>> + uint8_t *payload_out, size_t *len_out,
>> + CXLCCI *cci)
>> +{
>> + CXLMHSLDState *s = CXL_MHSLD(cci->d);
>> + MHDGetHeadInfoInput *input = (void *)payload_in;
>> + MHDGetHeadInfoOutput *output = (void *)payload_out;
>> + int i = 0;
>> +
>> + if (input->start_head > MHSLD_HEADS) {
>
>Should be ">=" ?
Right. Head index starts from 0, so this condition should be there.
>
>Also, per the spec, we need also check the "number of heads" field, if
>start_head and number of heads together point to a non-exist head, we
>should return invalid input.
Yes. Will add this check in V1 patch.
>
>> + return CXL_MBOX_INVALID_INPUT;
>> + }
>> +
>> + output->nr_heads = MIN((MHSLD_HEADS - input->start_head), input->nr_heads);
>> + for (i = input->start_head; i < input->start_head + output->nr_heads; i++) {
>> + output->head_info_list[i].port_number =
>> + s->mhd_state->head_info_blocks[i].port_number;
>> + output->head_info_list[i].max_link_width =
>> + s->mhd_state->head_info_blocks[i].max_link_width;
>> + output->head_info_list[i].nego_link_width =
>> + s->mhd_state->head_info_blocks[i].nego_link_width;
>> + output->head_info_list[i].supp_link_speeds_vector =
>> + s->mhd_state->head_info_blocks[i].supp_link_speeds_vector;
>> + output->head_info_list[i].max_link_speed =
>> + s->mhd_state->head_info_blocks[i].max_link_speed;
>> + output->head_info_list[i].current_link_speed =
>> + s->mhd_state->head_info_blocks[i].current_link_speed;
>> + output->head_info_list[i].ltssm_state =
>> + s->mhd_state->head_info_blocks[i].ltssm_state;
>> + output->head_info_list[i].first_nego_lane_num =
>> + s->mhd_state->head_info_blocks[i].first_nego_lane_num;
>> + output->head_info_list[i].link_state_flags =
>> + s->mhd_state->head_info_blocks[i].link_state_flags;
>> + }
>> +
>> + *len_out = sizeof(*output) + output->nr_heads * sizeof(MHDHeadInfoBlock);
>> + return CXL_MBOX_SUCCESS;
>> +}
>> +
>> static const struct cxl_cmd cxl_cmd_set_mhsld[256][256] = {
>> [MHSLD_MHD][GET_MHD_INFO] = {"GET_MULTI_HEADED_INFO",
>> cmd_mhd_get_info, 2, 0},
>> + [MHSLD_MHD][GET_MHD_HEAD_INFO] = {"GET_HEAD_INFO",
>> + cmd_mhd_get_head_info, 2, 0},
>> };
>>
>> static const Property cxl_mhsld_props[] = {
>> @@ -166,6 +214,47 @@ static void cxl_mhsld_state_initialize(CXLMHSLDState *s, size_t dc_size)
>> s->mhd_state->nr_blocks = dc_size / MHSLD_BLOCK_SZ;
>> }
>>
>> +
>> +static void cxl_mhsld_init_head_info(CXLMHSLDState *s, PCIDevice *pdev)
>> +{
>> + uint16_t lnksta = 0;
>> + uint16_t current_link_speed = 0;
>> + uint16_t negotiated_link_width = 0;
>> + uint16_t lnkcap = 0, lnkcap2 = 0;
>> + uint16_t max_link_width = 0;
>> + uint16_t max_link_speed = 0;
>> + uint16_t supported_link_speeds_vector = 0;
>> +
>> + lnksta = pdev->config_read(pdev,
>> + pdev->exp.exp_cap + PCI_EXP_LNKSTA,
>> + sizeof(lnksta));
>> + lnkcap = pdev->config_read(pdev,
>> + pdev->exp.exp_cap + PCI_EXP_LNKCAP,
>> + sizeof(lnkcap));
>> + lnkcap2 = pdev->config_read(pdev,
>> + pdev->exp.exp_cap + PCI_EXP_LNKCAP2,
>> + sizeof(lnkcap2));
>> + supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
>> + max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
>> + max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
>> + current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
>> + negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
>> +
>> + s->mhd_state->head_info_blocks[s->mhd_head].port_number = s->mhd_head;
>> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_width = max_link_width;
>> + s->mhd_state->head_info_blocks[s->mhd_head].nego_link_width =
>> + negotiated_link_width;
>> + s->mhd_state->head_info_blocks[s->mhd_head].supp_link_speeds_vector =
>> + supported_link_speeds_vector;
>> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_speed =
>> + max_link_speed;
>> + s->mhd_state->head_info_blocks[s->mhd_head].current_link_speed =
>> + current_link_speed;
>> + s->mhd_state->head_info_blocks[s->mhd_head].ltssm_state = 0x7;
>> + s->mhd_state->head_info_blocks[s->mhd_head].first_nego_lane_num = 0;
>> + s->mhd_state->head_info_blocks[s->mhd_head].link_state_flags = 0;
>> +}
>> +
>> /* Returns starting index of region in MHD map. */
>> static inline size_t cxl_mhsld_find_dc_region_start(PCIDevice *d,
>> CXLDCRegion *r)
>> @@ -376,7 +465,7 @@ static void cxl_mhsld_realize(PCIDevice *pci_dev, Error **errp)
>> }
>>
>> cxl_mhsld_state_initialize(s, dc_size);
>> -
>> + cxl_mhsld_init_head_info(s, pci_dev);
>> /* Set the LD ownership for this head to this system */
>> s->mhd_state->ldmap[s->mhd_head] = s->mhd_head;
>> return;
>> @@ -428,6 +517,7 @@ static void cxl_mhsld_class_init(ObjectClass *klass, void *data)
>>
>> CXLType3Class *cvc = CXL_TYPE3_CLASS(klass);
>> cvc->mhd_get_info = cmd_mhd_get_info;
>> + cvc->mhd_get_head_info = cmd_mhd_get_head_info;
>> cvc->mhd_access_valid = cxl_mhsld_access_valid;
>> cvc->mhd_reserve_extents = cxl_mhsld_reserve_extents;
>> cvc->mhd_reclaim_extents = cxl_mhsld_reclaim_extents;
>> diff --git a/hw/cxl/mhsld/mhsld.h b/hw/cxl/mhsld/mhsld.h
>> index e7ead1f0d2..c9fbec71ca 100644
>> --- a/hw/cxl/mhsld/mhsld.h
>> +++ b/hw/cxl/mhsld/mhsld.h
>> @@ -23,6 +23,18 @@
>> */
>> #define MHSLD_HEADS (8)
>>
>> +typedef struct MHDHeadInfoBlock {
>> + uint8_t port_number;
>> + uint8_t max_link_width;
>> + uint8_t nego_link_width;
>> + uint8_t supp_link_speeds_vector;
>> + uint8_t max_link_speed;
>> + uint8_t current_link_speed;
>> + uint8_t ltssm_state;
>> + uint8_t first_nego_lane_num;
>> + uint8_t link_state_flags;
>> +} QEMU_PACKED MHDHeadInfoBlock;
>> +
>> /*
>> * The shared state cannot have 2 variable sized regions
>> * so we have to max out the ldmap.
>> @@ -32,6 +44,7 @@ typedef struct MHSLDSharedState {
>> uint8_t nr_lds;
>> uint8_t ldmap[MHSLD_HEADS];
>> uint64_t nr_blocks;
>> + MHDHeadInfoBlock head_info_blocks[MHSLD_HEADS];
>> uint8_t blocks[];
>> } MHSLDSharedState;
>>
>> @@ -52,6 +65,7 @@ struct CXLMHSLDClass {
>> enum {
>> MHSLD_MHD = 0x55,
>> #define GET_MHD_INFO 0x0
>> + #define GET_MHD_HEAD_INFO 0x1
>> };
>>
>> /*
>> @@ -72,4 +86,16 @@ typedef struct MHDGetInfoOutput {
>> uint16_t resv2;
>> uint8_t ldmap[];
>> } QEMU_PACKED MHDGetInfoOutput;
>> +
>> +typedef struct MHDGetHeadInfoInput {
>> + uint8_t start_head;
>> + uint8_t nr_heads;
>> +} QEMU_PACKED MHDGetHeadInfoInput;
>> +
>> +typedef struct MHDGetHeadInfoOutput {
>> + uint8_t nr_heads;
>> + uint8_t rsvd[3];
>> + MHDHeadInfoBlock head_info_list[];
>
>s/head_info_list/head_info/
Okay. Will use head_info in V1 patch.
>
>Fan
Thanks Fan Ni for reviewing the patch.
Alok
>
>> +} QEMU_PACKED MHDGetHeadInfoOutput;
>> +
>> #endif
>> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
>> index ca515cab13..c93c71c45d 100644
>> --- a/include/hw/cxl/cxl_device.h
>> +++ b/include/hw/cxl/cxl_device.h
>> @@ -732,6 +732,12 @@ struct CXLType3Class {
>> uint8_t *payload_out,
>> size_t *len_out,
>> CXLCCI *cci);
>> + CXLRetCode (*mhd_get_head_info)(const struct cxl_cmd *cmd,
>> + uint8_t *payload_in,
>> + size_t len_in,
>> + uint8_t *payload_out,
>> + size_t *len_out,
>> + CXLCCI *cci);
>> bool (*mhd_access_valid)(PCIDevice *d, uint64_t addr, unsigned int size);
>> bool (*mhd_reserve_extents)(PCIDevice *d,
>> CxlDynamicCapacityExtentList *records,
>> --
>> 2.34.1
>>
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h)
2025-05-22 6:31 ` [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h) Vinayak Holikatti
2025-05-22 16:48 ` Fan Ni
@ 2025-05-22 19:15 ` Davidlohr Bueso
2025-07-01 13:05 ` Alok Rathore
2025-05-22 23:12 ` Anisa Su
2025-05-29 12:13 ` Jonathan Cameron via
3 siblings, 1 reply; 10+ messages in thread
From: Davidlohr Bueso @ 2025-05-22 19:15 UTC (permalink / raw)
To: Vinayak Holikatti, Jonathan.Cameron
Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, vishak.g, krish.reddy,
a.manzanares, alok.rathore
On Thu, 22 May 2025, Vinayak Holikatti wrote:
>CXL spec 3.2 section 7.6.7.5.2 describes Get Head Info.
So how was this tested? Ideally this now comes with a libcxlmi (or equivalent)
test case. See for example how Anisa is going about this with the FMAPI DCD
patches:
https://lore.kernel.org/linux-cxl/20250508001754.122180-1-anisa.su887@gmail.com/
>Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com>
>---
>This patch is generated against Jonathan Cameron's branch cxl-2025-03-20
Adding him to the thread :)
>
> hw/cxl/cxl-mailbox-utils.c | 21 +++++++++
> hw/cxl/mhsld/mhsld.c | 92 ++++++++++++++++++++++++++++++++++++-
> hw/cxl/mhsld/mhsld.h | 26 +++++++++++
> include/hw/cxl/cxl_device.h | 6 +++
> 4 files changed, 144 insertions(+), 1 deletion(-)
>
>diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>index a02d130926..4f25caecea 100644
>--- a/hw/cxl/cxl-mailbox-utils.c
>+++ b/hw/cxl/cxl-mailbox-utils.c
>@@ -122,6 +122,7 @@ enum {
> #define MANAGEMENT_COMMAND 0x0
> MHD = 0x55,
> #define GET_MHD_INFO 0x0
>+ #define GET_MHD_HEAD_INFO 0x1
> };
>
> /* CCI Message Format CXL r3.1 Figure 7-19 */
>@@ -267,6 +268,23 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
> return CXL_MBOX_UNSUPPORTED;
> }
>
>+/*
>+ * CXL r3.2 section 7.6.7.5.2 - Get Multi-Headed Head Info (Opcode 5501h)
>+ */
>+static CXLRetCode cmd_mhd_get_head_info(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);
>+ CXLType3Class *cvc = CXL_TYPE3_GET_CLASS(ct3d);
>+ if (cvc->mhd_get_head_info) {
>+ return cvc->mhd_get_head_info(cmd, payload_in, len_in, payload_out,
>+ len_out, cci);
>+ }
>+ return CXL_MBOX_UNSUPPORTED;
>+}
>+
> static CXLRetCode cmd_events_get_records(const struct cxl_cmd *cmd,
> uint8_t *payload_in, size_t len_in,
> uint8_t *payload_out, size_t *len_out,
>@@ -3429,6 +3447,7 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> "MEDIA_AND_POISON_GET_SCAN_MEDIA_RESULTS",
> cmd_media_get_scan_media_results, 0, 0 },
> [MHD][GET_MHD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_info, 2, 0},
>+ [MHD][GET_MHD_HEAD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_head_info, 2, 0},
> };
>
> static const struct cxl_cmd cxl_cmd_set_dcd[256][256] = {
>@@ -3761,6 +3780,8 @@ static const struct cxl_cmd cxl_cmd_set_t3_fm_owned_ld_mctp[256][256] = {
> [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
> [TUNNEL][MANAGEMENT_COMMAND] = { "TUNNEL_MANAGEMENT_COMMAND",
> cmd_tunnel_management_cmd, ~0, 0 },
>+ [MHD][GET_MHD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_info, 2, 0},
>+ [MHD][GET_MHD_HEAD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_head_info, 2, 0},
> };
>
> void cxl_initialize_t3_fm_owned_ld_mctpcci(CXLCCI *cci, DeviceState *d,
>diff --git a/hw/cxl/mhsld/mhsld.c b/hw/cxl/mhsld/mhsld.c
>index 9f633b3bed..981546b5ff 100644
>--- a/hw/cxl/mhsld/mhsld.c
>+++ b/hw/cxl/mhsld/mhsld.c
>@@ -61,9 +61,57 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
>+/*
>+ * CXL r3.2 section 7.6.7.5.2 - Get Head Info (Opcode 5501h)
>+ *
>+ * This command retrieves the number of heads, number of supported LDs,
>+ * and Head-to-LD mapping of a Multi-Headed device.
>+ */
>+static CXLRetCode cmd_mhd_get_head_info(const struct cxl_cmd *cmd,
>+ uint8_t *payload_in, size_t len_in,
>+ uint8_t *payload_out, size_t *len_out,
>+ CXLCCI *cci)
>+{
>+ CXLMHSLDState *s = CXL_MHSLD(cci->d);
>+ MHDGetHeadInfoInput *input = (void *)payload_in;
>+ MHDGetHeadInfoOutput *output = (void *)payload_out;
>+ int i = 0;
>+
>+ if (input->start_head > MHSLD_HEADS) {
>+ return CXL_MBOX_INVALID_INPUT;
>+ }
>+
>+ output->nr_heads = MIN((MHSLD_HEADS - input->start_head), input->nr_heads);
>+ for (i = input->start_head; i < input->start_head + output->nr_heads; i++) {
>+ output->head_info_list[i].port_number =
>+ s->mhd_state->head_info_blocks[i].port_number;
>+ output->head_info_list[i].max_link_width =
>+ s->mhd_state->head_info_blocks[i].max_link_width;
>+ output->head_info_list[i].nego_link_width =
>+ s->mhd_state->head_info_blocks[i].nego_link_width;
>+ output->head_info_list[i].supp_link_speeds_vector =
>+ s->mhd_state->head_info_blocks[i].supp_link_speeds_vector;
>+ output->head_info_list[i].max_link_speed =
>+ s->mhd_state->head_info_blocks[i].max_link_speed;
>+ output->head_info_list[i].current_link_speed =
>+ s->mhd_state->head_info_blocks[i].current_link_speed;
>+ output->head_info_list[i].ltssm_state =
>+ s->mhd_state->head_info_blocks[i].ltssm_state;
>+ output->head_info_list[i].first_nego_lane_num =
>+ s->mhd_state->head_info_blocks[i].first_nego_lane_num;
>+ output->head_info_list[i].link_state_flags =
>+ s->mhd_state->head_info_blocks[i].link_state_flags;
>+ }
>+
>+ *len_out = sizeof(*output) + output->nr_heads * sizeof(MHDHeadInfoBlock);
>+ return CXL_MBOX_SUCCESS;
>+}
>+
> static const struct cxl_cmd cxl_cmd_set_mhsld[256][256] = {
> [MHSLD_MHD][GET_MHD_INFO] = {"GET_MULTI_HEADED_INFO",
> cmd_mhd_get_info, 2, 0},
>+ [MHSLD_MHD][GET_MHD_HEAD_INFO] = {"GET_HEAD_INFO",
>+ cmd_mhd_get_head_info, 2, 0},
> };
>
> static const Property cxl_mhsld_props[] = {
>@@ -166,6 +214,47 @@ static void cxl_mhsld_state_initialize(CXLMHSLDState *s, size_t dc_size)
> s->mhd_state->nr_blocks = dc_size / MHSLD_BLOCK_SZ;
> }
>
>+
>+static void cxl_mhsld_init_head_info(CXLMHSLDState *s, PCIDevice *pdev)
>+{
>+ uint16_t lnksta = 0;
>+ uint16_t current_link_speed = 0;
>+ uint16_t negotiated_link_width = 0;
>+ uint16_t lnkcap = 0, lnkcap2 = 0;
>+ uint16_t max_link_width = 0;
>+ uint16_t max_link_speed = 0;
>+ uint16_t supported_link_speeds_vector = 0;
>+
>+ lnksta = pdev->config_read(pdev,
>+ pdev->exp.exp_cap + PCI_EXP_LNKSTA,
>+ sizeof(lnksta));
>+ lnkcap = pdev->config_read(pdev,
>+ pdev->exp.exp_cap + PCI_EXP_LNKCAP,
>+ sizeof(lnkcap));
>+ lnkcap2 = pdev->config_read(pdev,
>+ pdev->exp.exp_cap + PCI_EXP_LNKCAP2,
>+ sizeof(lnkcap2));
>+ supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
>+ max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
>+ max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
>+ current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
>+ negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
>+
>+ s->mhd_state->head_info_blocks[s->mhd_head].port_number = s->mhd_head;
>+ s->mhd_state->head_info_blocks[s->mhd_head].max_link_width = max_link_width;
>+ s->mhd_state->head_info_blocks[s->mhd_head].nego_link_width =
>+ negotiated_link_width;
>+ s->mhd_state->head_info_blocks[s->mhd_head].supp_link_speeds_vector =
>+ supported_link_speeds_vector;
>+ s->mhd_state->head_info_blocks[s->mhd_head].max_link_speed =
>+ max_link_speed;
>+ s->mhd_state->head_info_blocks[s->mhd_head].current_link_speed =
>+ current_link_speed;
>+ s->mhd_state->head_info_blocks[s->mhd_head].ltssm_state = 0x7;
>+ s->mhd_state->head_info_blocks[s->mhd_head].first_nego_lane_num = 0;
>+ s->mhd_state->head_info_blocks[s->mhd_head].link_state_flags = 0;
>+}
>+
> /* Returns starting index of region in MHD map. */
> static inline size_t cxl_mhsld_find_dc_region_start(PCIDevice *d,
> CXLDCRegion *r)
>@@ -376,7 +465,7 @@ static void cxl_mhsld_realize(PCIDevice *pci_dev, Error **errp)
> }
>
> cxl_mhsld_state_initialize(s, dc_size);
>-
>+ cxl_mhsld_init_head_info(s, pci_dev);
> /* Set the LD ownership for this head to this system */
> s->mhd_state->ldmap[s->mhd_head] = s->mhd_head;
> return;
>@@ -428,6 +517,7 @@ static void cxl_mhsld_class_init(ObjectClass *klass, void *data)
>
> CXLType3Class *cvc = CXL_TYPE3_CLASS(klass);
> cvc->mhd_get_info = cmd_mhd_get_info;
>+ cvc->mhd_get_head_info = cmd_mhd_get_head_info;
> cvc->mhd_access_valid = cxl_mhsld_access_valid;
> cvc->mhd_reserve_extents = cxl_mhsld_reserve_extents;
> cvc->mhd_reclaim_extents = cxl_mhsld_reclaim_extents;
>diff --git a/hw/cxl/mhsld/mhsld.h b/hw/cxl/mhsld/mhsld.h
>index e7ead1f0d2..c9fbec71ca 100644
>--- a/hw/cxl/mhsld/mhsld.h
>+++ b/hw/cxl/mhsld/mhsld.h
>@@ -23,6 +23,18 @@
> */
> #define MHSLD_HEADS (8)
>
>+typedef struct MHDHeadInfoBlock {
>+ uint8_t port_number;
>+ uint8_t max_link_width;
>+ uint8_t nego_link_width;
>+ uint8_t supp_link_speeds_vector;
>+ uint8_t max_link_speed;
>+ uint8_t current_link_speed;
>+ uint8_t ltssm_state;
>+ uint8_t first_nego_lane_num;
>+ uint8_t link_state_flags;
>+} QEMU_PACKED MHDHeadInfoBlock;
>+
> /*
> * The shared state cannot have 2 variable sized regions
> * so we have to max out the ldmap.
>@@ -32,6 +44,7 @@ typedef struct MHSLDSharedState {
> uint8_t nr_lds;
> uint8_t ldmap[MHSLD_HEADS];
> uint64_t nr_blocks;
>+ MHDHeadInfoBlock head_info_blocks[MHSLD_HEADS];
> uint8_t blocks[];
> } MHSLDSharedState;
>
>@@ -52,6 +65,7 @@ struct CXLMHSLDClass {
> enum {
> MHSLD_MHD = 0x55,
> #define GET_MHD_INFO 0x0
>+ #define GET_MHD_HEAD_INFO 0x1
> };
>
> /*
>@@ -72,4 +86,16 @@ typedef struct MHDGetInfoOutput {
> uint16_t resv2;
> uint8_t ldmap[];
> } QEMU_PACKED MHDGetInfoOutput;
>+
>+typedef struct MHDGetHeadInfoInput {
>+ uint8_t start_head;
>+ uint8_t nr_heads;
>+} QEMU_PACKED MHDGetHeadInfoInput;
>+
>+typedef struct MHDGetHeadInfoOutput {
>+ uint8_t nr_heads;
>+ uint8_t rsvd[3];
>+ MHDHeadInfoBlock head_info_list[];
>+} QEMU_PACKED MHDGetHeadInfoOutput;
>+
> #endif
>diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
>index ca515cab13..c93c71c45d 100644
>--- a/include/hw/cxl/cxl_device.h
>+++ b/include/hw/cxl/cxl_device.h
>@@ -732,6 +732,12 @@ struct CXLType3Class {
> uint8_t *payload_out,
> size_t *len_out,
> CXLCCI *cci);
>+ CXLRetCode (*mhd_get_head_info)(const struct cxl_cmd *cmd,
>+ uint8_t *payload_in,
>+ size_t len_in,
>+ uint8_t *payload_out,
>+ size_t *len_out,
>+ CXLCCI *cci);
> bool (*mhd_access_valid)(PCIDevice *d, uint64_t addr, unsigned int size);
> bool (*mhd_reserve_extents)(PCIDevice *d,
> CxlDynamicCapacityExtentList *records,
>--
>2.34.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h)
2025-05-22 19:15 ` Davidlohr Bueso
@ 2025-07-01 13:05 ` Alok Rathore
0 siblings, 0 replies; 10+ messages in thread
From: Alok Rathore @ 2025-07-01 13:05 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: Jonathan.Cameron, qemu-devel, gost.dev, linux-cxl, nifan.cxl,
vishak.g, krish.reddy, a.manzanares, alokrathore20
[-- Attachment #1: Type: text/plain, Size: 12007 bytes --]
On 22/05/25 12:15PM, Davidlohr Bueso wrote:
>On Thu, 22 May 2025, Vinayak Holikatti wrote:
>
>>CXL spec 3.2 section 7.6.7.5.2 describes Get Head Info.
>
>So how was this tested? Ideally this now comes with a libcxlmi (or equivalent)
>test case. See for example how Anisa is going about this with the FMAPI DCD
>patches:
>
>https://lore.kernel.org/linux-cxl/20250508001754.122180-1-anisa.su887@gmail.com/
Used libcxl-mi to test this feature. Will update the information in V1 patch.
>
>>Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com>
>>---
>>This patch is generated against Jonathan Cameron's branch cxl-2025-03-20
>
>Adding him to the thread :)
>
>>
>>hw/cxl/cxl-mailbox-utils.c | 21 +++++++++
>>hw/cxl/mhsld/mhsld.c | 92 ++++++++++++++++++++++++++++++++++++-
>>hw/cxl/mhsld/mhsld.h | 26 +++++++++++
>>include/hw/cxl/cxl_device.h | 6 +++
>>4 files changed, 144 insertions(+), 1 deletion(-)
>>
>>diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>>index a02d130926..4f25caecea 100644
>>--- a/hw/cxl/cxl-mailbox-utils.c
>>+++ b/hw/cxl/cxl-mailbox-utils.c
>>@@ -122,6 +122,7 @@ enum {
>> #define MANAGEMENT_COMMAND 0x0
>> MHD = 0x55,
>> #define GET_MHD_INFO 0x0
>>+ #define GET_MHD_HEAD_INFO 0x1
>>};
>>
>>/* CCI Message Format CXL r3.1 Figure 7-19 */
>>@@ -267,6 +268,23 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
>> return CXL_MBOX_UNSUPPORTED;
>>}
>>
>>+/*
>>+ * CXL r3.2 section 7.6.7.5.2 - Get Multi-Headed Head Info (Opcode 5501h)
>>+ */
>>+static CXLRetCode cmd_mhd_get_head_info(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);
>>+ CXLType3Class *cvc = CXL_TYPE3_GET_CLASS(ct3d);
>>+ if (cvc->mhd_get_head_info) {
>>+ return cvc->mhd_get_head_info(cmd, payload_in, len_in, payload_out,
>>+ len_out, cci);
>>+ }
>>+ return CXL_MBOX_UNSUPPORTED;
>>+}
>>+
>>static CXLRetCode cmd_events_get_records(const struct cxl_cmd *cmd,
>> uint8_t *payload_in, size_t len_in,
>> uint8_t *payload_out, size_t *len_out,
>>@@ -3429,6 +3447,7 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
>> "MEDIA_AND_POISON_GET_SCAN_MEDIA_RESULTS",
>> cmd_media_get_scan_media_results, 0, 0 },
>> [MHD][GET_MHD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_info, 2, 0},
>>+ [MHD][GET_MHD_HEAD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_head_info, 2, 0},
>>};
>>
>>static const struct cxl_cmd cxl_cmd_set_dcd[256][256] = {
>>@@ -3761,6 +3780,8 @@ static const struct cxl_cmd cxl_cmd_set_t3_fm_owned_ld_mctp[256][256] = {
>> [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
>> [TUNNEL][MANAGEMENT_COMMAND] = { "TUNNEL_MANAGEMENT_COMMAND",
>> cmd_tunnel_management_cmd, ~0, 0 },
>>+ [MHD][GET_MHD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_info, 2, 0},
>>+ [MHD][GET_MHD_HEAD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_head_info, 2, 0},
>>};
>>
>>void cxl_initialize_t3_fm_owned_ld_mctpcci(CXLCCI *cci, DeviceState *d,
>>diff --git a/hw/cxl/mhsld/mhsld.c b/hw/cxl/mhsld/mhsld.c
>>index 9f633b3bed..981546b5ff 100644
>>--- a/hw/cxl/mhsld/mhsld.c
>>+++ b/hw/cxl/mhsld/mhsld.c
>>@@ -61,9 +61,57 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
>> return CXL_MBOX_SUCCESS;
>>}
>>
>>+/*
>>+ * CXL r3.2 section 7.6.7.5.2 - Get Head Info (Opcode 5501h)
>>+ *
>>+ * This command retrieves the number of heads, number of supported LDs,
>>+ * and Head-to-LD mapping of a Multi-Headed device.
>>+ */
>>+static CXLRetCode cmd_mhd_get_head_info(const struct cxl_cmd *cmd,
>>+ uint8_t *payload_in, size_t len_in,
>>+ uint8_t *payload_out, size_t *len_out,
>>+ CXLCCI *cci)
>>+{
>>+ CXLMHSLDState *s = CXL_MHSLD(cci->d);
>>+ MHDGetHeadInfoInput *input = (void *)payload_in;
>>+ MHDGetHeadInfoOutput *output = (void *)payload_out;
>>+ int i = 0;
>>+
>>+ if (input->start_head > MHSLD_HEADS) {
>>+ return CXL_MBOX_INVALID_INPUT;
>>+ }
>>+
>>+ output->nr_heads = MIN((MHSLD_HEADS - input->start_head), input->nr_heads);
>>+ for (i = input->start_head; i < input->start_head + output->nr_heads; i++) {
>>+ output->head_info_list[i].port_number =
>>+ s->mhd_state->head_info_blocks[i].port_number;
>>+ output->head_info_list[i].max_link_width =
>>+ s->mhd_state->head_info_blocks[i].max_link_width;
>>+ output->head_info_list[i].nego_link_width =
>>+ s->mhd_state->head_info_blocks[i].nego_link_width;
>>+ output->head_info_list[i].supp_link_speeds_vector =
>>+ s->mhd_state->head_info_blocks[i].supp_link_speeds_vector;
>>+ output->head_info_list[i].max_link_speed =
>>+ s->mhd_state->head_info_blocks[i].max_link_speed;
>>+ output->head_info_list[i].current_link_speed =
>>+ s->mhd_state->head_info_blocks[i].current_link_speed;
>>+ output->head_info_list[i].ltssm_state =
>>+ s->mhd_state->head_info_blocks[i].ltssm_state;
>>+ output->head_info_list[i].first_nego_lane_num =
>>+ s->mhd_state->head_info_blocks[i].first_nego_lane_num;
>>+ output->head_info_list[i].link_state_flags =
>>+ s->mhd_state->head_info_blocks[i].link_state_flags;
>>+ }
>>+
>>+ *len_out = sizeof(*output) + output->nr_heads * sizeof(MHDHeadInfoBlock);
>>+ return CXL_MBOX_SUCCESS;
>>+}
>>+
>>static const struct cxl_cmd cxl_cmd_set_mhsld[256][256] = {
>> [MHSLD_MHD][GET_MHD_INFO] = {"GET_MULTI_HEADED_INFO",
>> cmd_mhd_get_info, 2, 0},
>>+ [MHSLD_MHD][GET_MHD_HEAD_INFO] = {"GET_HEAD_INFO",
>>+ cmd_mhd_get_head_info, 2, 0},
>>};
>>
>>static const Property cxl_mhsld_props[] = {
>>@@ -166,6 +214,47 @@ static void cxl_mhsld_state_initialize(CXLMHSLDState *s, size_t dc_size)
>> s->mhd_state->nr_blocks = dc_size / MHSLD_BLOCK_SZ;
>>}
>>
>>+
>>+static void cxl_mhsld_init_head_info(CXLMHSLDState *s, PCIDevice *pdev)
>>+{
>>+ uint16_t lnksta = 0;
>>+ uint16_t current_link_speed = 0;
>>+ uint16_t negotiated_link_width = 0;
>>+ uint16_t lnkcap = 0, lnkcap2 = 0;
>>+ uint16_t max_link_width = 0;
>>+ uint16_t max_link_speed = 0;
>>+ uint16_t supported_link_speeds_vector = 0;
>>+
>>+ lnksta = pdev->config_read(pdev,
>>+ pdev->exp.exp_cap + PCI_EXP_LNKSTA,
>>+ sizeof(lnksta));
>>+ lnkcap = pdev->config_read(pdev,
>>+ pdev->exp.exp_cap + PCI_EXP_LNKCAP,
>>+ sizeof(lnkcap));
>>+ lnkcap2 = pdev->config_read(pdev,
>>+ pdev->exp.exp_cap + PCI_EXP_LNKCAP2,
>>+ sizeof(lnkcap2));
>>+ supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
>>+ max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
>>+ max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
>>+ current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
>>+ negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
>>+
>>+ s->mhd_state->head_info_blocks[s->mhd_head].port_number = s->mhd_head;
>>+ s->mhd_state->head_info_blocks[s->mhd_head].max_link_width = max_link_width;
>>+ s->mhd_state->head_info_blocks[s->mhd_head].nego_link_width =
>>+ negotiated_link_width;
>>+ s->mhd_state->head_info_blocks[s->mhd_head].supp_link_speeds_vector =
>>+ supported_link_speeds_vector;
>>+ s->mhd_state->head_info_blocks[s->mhd_head].max_link_speed =
>>+ max_link_speed;
>>+ s->mhd_state->head_info_blocks[s->mhd_head].current_link_speed =
>>+ current_link_speed;
>>+ s->mhd_state->head_info_blocks[s->mhd_head].ltssm_state = 0x7;
>>+ s->mhd_state->head_info_blocks[s->mhd_head].first_nego_lane_num = 0;
>>+ s->mhd_state->head_info_blocks[s->mhd_head].link_state_flags = 0;
>>+}
>>+
>>/* Returns starting index of region in MHD map. */
>>static inline size_t cxl_mhsld_find_dc_region_start(PCIDevice *d,
>> CXLDCRegion *r)
>>@@ -376,7 +465,7 @@ static void cxl_mhsld_realize(PCIDevice *pci_dev, Error **errp)
>> }
>>
>> cxl_mhsld_state_initialize(s, dc_size);
>>-
>>+ cxl_mhsld_init_head_info(s, pci_dev);
>> /* Set the LD ownership for this head to this system */
>> s->mhd_state->ldmap[s->mhd_head] = s->mhd_head;
>> return;
>>@@ -428,6 +517,7 @@ static void cxl_mhsld_class_init(ObjectClass *klass, void *data)
>>
>> CXLType3Class *cvc = CXL_TYPE3_CLASS(klass);
>> cvc->mhd_get_info = cmd_mhd_get_info;
>>+ cvc->mhd_get_head_info = cmd_mhd_get_head_info;
>> cvc->mhd_access_valid = cxl_mhsld_access_valid;
>> cvc->mhd_reserve_extents = cxl_mhsld_reserve_extents;
>> cvc->mhd_reclaim_extents = cxl_mhsld_reclaim_extents;
>>diff --git a/hw/cxl/mhsld/mhsld.h b/hw/cxl/mhsld/mhsld.h
>>index e7ead1f0d2..c9fbec71ca 100644
>>--- a/hw/cxl/mhsld/mhsld.h
>>+++ b/hw/cxl/mhsld/mhsld.h
>>@@ -23,6 +23,18 @@
>> */
>>#define MHSLD_HEADS (8)
>>
>>+typedef struct MHDHeadInfoBlock {
>>+ uint8_t port_number;
>>+ uint8_t max_link_width;
>>+ uint8_t nego_link_width;
>>+ uint8_t supp_link_speeds_vector;
>>+ uint8_t max_link_speed;
>>+ uint8_t current_link_speed;
>>+ uint8_t ltssm_state;
>>+ uint8_t first_nego_lane_num;
>>+ uint8_t link_state_flags;
>>+} QEMU_PACKED MHDHeadInfoBlock;
>>+
>>/*
>> * The shared state cannot have 2 variable sized regions
>> * so we have to max out the ldmap.
>>@@ -32,6 +44,7 @@ typedef struct MHSLDSharedState {
>> uint8_t nr_lds;
>> uint8_t ldmap[MHSLD_HEADS];
>> uint64_t nr_blocks;
>>+ MHDHeadInfoBlock head_info_blocks[MHSLD_HEADS];
>> uint8_t blocks[];
>>} MHSLDSharedState;
>>
>>@@ -52,6 +65,7 @@ struct CXLMHSLDClass {
>>enum {
>> MHSLD_MHD = 0x55,
>> #define GET_MHD_INFO 0x0
>>+ #define GET_MHD_HEAD_INFO 0x1
>>};
>>
>>/*
>>@@ -72,4 +86,16 @@ typedef struct MHDGetInfoOutput {
>> uint16_t resv2;
>> uint8_t ldmap[];
>>} QEMU_PACKED MHDGetInfoOutput;
>>+
>>+typedef struct MHDGetHeadInfoInput {
>>+ uint8_t start_head;
>>+ uint8_t nr_heads;
>>+} QEMU_PACKED MHDGetHeadInfoInput;
>>+
>>+typedef struct MHDGetHeadInfoOutput {
>>+ uint8_t nr_heads;
>>+ uint8_t rsvd[3];
>>+ MHDHeadInfoBlock head_info_list[];
>>+} QEMU_PACKED MHDGetHeadInfoOutput;
>>+
>>#endif
>>diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
>>index ca515cab13..c93c71c45d 100644
>>--- a/include/hw/cxl/cxl_device.h
>>+++ b/include/hw/cxl/cxl_device.h
>>@@ -732,6 +732,12 @@ struct CXLType3Class {
>> uint8_t *payload_out,
>> size_t *len_out,
>> CXLCCI *cci);
>>+ CXLRetCode (*mhd_get_head_info)(const struct cxl_cmd *cmd,
>>+ uint8_t *payload_in,
>>+ size_t len_in,
>>+ uint8_t *payload_out,
>>+ size_t *len_out,
>>+ CXLCCI *cci);
>> bool (*mhd_access_valid)(PCIDevice *d, uint64_t addr, unsigned int size);
>> bool (*mhd_reserve_extents)(PCIDevice *d,
>> CxlDynamicCapacityExtentList *records,
>>--
>>2.34.1
>>
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h)
2025-05-22 6:31 ` [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h) Vinayak Holikatti
2025-05-22 16:48 ` Fan Ni
2025-05-22 19:15 ` Davidlohr Bueso
@ 2025-05-22 23:12 ` Anisa Su
2025-07-01 13:11 ` Alok Rathore
2025-05-29 12:13 ` Jonathan Cameron via
3 siblings, 1 reply; 10+ messages in thread
From: Anisa Su @ 2025-05-22 23:12 UTC (permalink / raw)
To: Vinayak Holikatti
Cc: nifan.cxl, a.manzanares, alok.rathore, dave, gost.dev,
krish.reddy, linux-cxl, qemu-devel, vishak.g
On Thu, May 22, 2025 at 12:01:35PM +0530, Vinayak Holikatti wrote:
> CXL spec 3.2 section 7.6.7.5.2 describes Get Head Info.
>
> Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com>
> ---
> This patch is generated against Jonathan Cameron's branch cxl-2025-03-20
>
> hw/cxl/cxl-mailbox-utils.c | 21 +++++++++
> hw/cxl/mhsld/mhsld.c | 92 ++++++++++++++++++++++++++++++++++++-
> hw/cxl/mhsld/mhsld.h | 26 +++++++++++
> include/hw/cxl/cxl_device.h | 6 +++
> 4 files changed, 144 insertions(+), 1 deletion(-)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index a02d130926..4f25caecea 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -122,6 +122,7 @@ enum {
> #define MANAGEMENT_COMMAND 0x0
> MHD = 0x55,
> #define GET_MHD_INFO 0x0
> + #define GET_MHD_HEAD_INFO 0x1
> };
>
> /* CCI Message Format CXL r3.1 Figure 7-19 */
> @@ -267,6 +268,23 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
> return CXL_MBOX_UNSUPPORTED;
> }
>
> +/*
> + * CXL r3.2 section 7.6.7.5.2 - Get Multi-Headed Head Info (Opcode 5501h)
> + */
> +static CXLRetCode cmd_mhd_get_head_info(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);
> + CXLType3Class *cvc = CXL_TYPE3_GET_CLASS(ct3d);
> + if (cvc->mhd_get_head_info) {
> + return cvc->mhd_get_head_info(cmd, payload_in, len_in, payload_out,
> + len_out, cci);
> + }
> + return CXL_MBOX_UNSUPPORTED;
> +}
> +
> static CXLRetCode cmd_events_get_records(const struct cxl_cmd *cmd,
> uint8_t *payload_in, size_t len_in,
> uint8_t *payload_out, size_t *len_out,
> @@ -3429,6 +3447,7 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> "MEDIA_AND_POISON_GET_SCAN_MEDIA_RESULTS",
> cmd_media_get_scan_media_results, 0, 0 },
> [MHD][GET_MHD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_info, 2, 0},
> + [MHD][GET_MHD_HEAD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_head_info, 2, 0},
The name here "GET_MULTI_HEADED_INFO" is the same as the previous command.
> };
>
...
> +
> +static void cxl_mhsld_init_head_info(CXLMHSLDState *s, PCIDevice *pdev)
> +{
> + uint16_t lnksta = 0;
> + uint16_t current_link_speed = 0;
> + uint16_t negotiated_link_width = 0;
> + uint16_t lnkcap = 0, lnkcap2 = 0;
> + uint16_t max_link_width = 0;
> + uint16_t max_link_speed = 0;
> + uint16_t supported_link_speeds_vector = 0;
> +
> + lnksta = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKSTA,
> + sizeof(lnksta));
> + lnkcap = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKCAP,
> + sizeof(lnkcap));
> + lnkcap2 = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKCAP2,
> + sizeof(lnkcap2));
> + supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
> + max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
> + max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
> + current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
> + negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
> +
> + s->mhd_state->head_info_blocks[s->mhd_head].port_number = s->mhd_head;
> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_width = max_link_width;
> + s->mhd_state->head_info_blocks[s->mhd_head].nego_link_width =
> + negotiated_link_width;
> + s->mhd_state->head_info_blocks[s->mhd_head].supp_link_speeds_vector =
> + supported_link_speeds_vector;
> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_speed =
> + max_link_speed;
> + s->mhd_state->head_info_blocks[s->mhd_head].current_link_speed =
> + current_link_speed;
> + s->mhd_state->head_info_blocks[s->mhd_head].ltssm_state = 0x7;
I think it would be helpful to make a define for the ltssm states?
> + s->mhd_state->head_info_blocks[s->mhd_head].first_nego_lane_num = 0;
> + s->mhd_state->head_info_blocks[s->mhd_head].link_state_flags = 0;
> +}
> +
> /* Returns starting index of region in MHD map. */
> static inline size_t cxl_mhsld_find_dc_region_start(PCIDevice *d,
> CXLDCRegion *r)
> @@ -376,7 +465,7 @@ static void cxl_mhsld_realize(PCIDevice *pci_dev, Error **errp)
> }
>
> cxl_mhsld_state_initialize(s, dc_size);
> -
> + cxl_mhsld_init_head_info(s, pci_dev);
> /* Set the LD ownership for this head to this system */
> s->mhd_state->ldmap[s->mhd_head] = s->mhd_head;
> return;
> @@ -428,6 +517,7 @@ static void cxl_mhsld_class_init(ObjectClass *klass, void *data)
>
> CXLType3Class *cvc = CXL_TYPE3_CLASS(klass);
> cvc->mhd_get_info = cmd_mhd_get_info;
> + cvc->mhd_get_head_info = cmd_mhd_get_head_info;
> cvc->mhd_access_valid = cxl_mhsld_access_valid;
> cvc->mhd_reserve_extents = cxl_mhsld_reserve_extents;
> cvc->mhd_reclaim_extents = cxl_mhsld_reclaim_extents;
> diff --git a/hw/cxl/mhsld/mhsld.h b/hw/cxl/mhsld/mhsld.h
> index e7ead1f0d2..c9fbec71ca 100644
> --- a/hw/cxl/mhsld/mhsld.h
> +++ b/hw/cxl/mhsld/mhsld.h
> @@ -23,6 +23,18 @@
> */
> #define MHSLD_HEADS (8)
>
> +typedef struct MHDHeadInfoBlock {
> + uint8_t port_number;
> + uint8_t max_link_width;
> + uint8_t nego_link_width;
> + uint8_t supp_link_speeds_vector;
> + uint8_t max_link_speed;
> + uint8_t current_link_speed;
> + uint8_t ltssm_state;
> + uint8_t first_nego_lane_num;
> + uint8_t link_state_flags;
> +} QEMU_PACKED MHDHeadInfoBlock;
> +
> /*
> * The shared state cannot have 2 variable sized regions
> * so we have to max out the ldmap.
> @@ -32,6 +44,7 @@ typedef struct MHSLDSharedState {
> uint8_t nr_lds;
> uint8_t ldmap[MHSLD_HEADS];
> uint64_t nr_blocks;
> + MHDHeadInfoBlock head_info_blocks[MHSLD_HEADS];
> uint8_t blocks[];
> } MHSLDSharedState;
>
> @@ -52,6 +65,7 @@ struct CXLMHSLDClass {
> enum {
> MHSLD_MHD = 0x55,
> #define GET_MHD_INFO 0x0
> + #define GET_MHD_HEAD_INFO 0x1
> };
>
> /*
> @@ -72,4 +86,16 @@ typedef struct MHDGetInfoOutput {
> uint16_t resv2;
> uint8_t ldmap[];
> } QEMU_PACKED MHDGetInfoOutput;
> +
> +typedef struct MHDGetHeadInfoInput {
> + uint8_t start_head;
> + uint8_t nr_heads;
> +} QEMU_PACKED MHDGetHeadInfoInput;
> +
> +typedef struct MHDGetHeadInfoOutput {
> + uint8_t nr_heads;
> + uint8_t rsvd[3];
> + MHDHeadInfoBlock head_info_list[];
> +} QEMU_PACKED MHDGetHeadInfoOutput;
> +
> #endif
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index ca515cab13..c93c71c45d 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -732,6 +732,12 @@ struct CXLType3Class {
> uint8_t *payload_out,
> size_t *len_out,
> CXLCCI *cci);
> + CXLRetCode (*mhd_get_head_info)(const struct cxl_cmd *cmd,
> + uint8_t *payload_in,
> + size_t len_in,
> + uint8_t *payload_out,
> + size_t *len_out,
> + CXLCCI *cci);
I think the indent here needs to be aligned with the open parentheses on
the first line of the function declaration
> bool (*mhd_access_valid)(PCIDevice *d, uint64_t addr, unsigned int size);
> bool (*mhd_reserve_extents)(PCIDevice *d,
> CxlDynamicCapacityExtentList *records,
> --
> 2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h)
2025-05-22 23:12 ` Anisa Su
@ 2025-07-01 13:11 ` Alok Rathore
0 siblings, 0 replies; 10+ messages in thread
From: Alok Rathore @ 2025-07-01 13:11 UTC (permalink / raw)
To: Anisa Su
Cc: nifan.cxl, a.manzanares, dave, gost.dev, krish.reddy, linux-cxl,
qemu-devel, vishak.g, alokrathore20
[-- Attachment #1: Type: text/plain, Size: 8555 bytes --]
On 22/05/25 11:12PM, Anisa Su wrote:
>On Thu, May 22, 2025 at 12:01:35PM +0530, Vinayak Holikatti wrote:
>> CXL spec 3.2 section 7.6.7.5.2 describes Get Head Info.
>>
>> Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com>
>> ---
>> This patch is generated against Jonathan Cameron's branch cxl-2025-03-20
>>
>> hw/cxl/cxl-mailbox-utils.c | 21 +++++++++
>> hw/cxl/mhsld/mhsld.c | 92 ++++++++++++++++++++++++++++++++++++-
>> hw/cxl/mhsld/mhsld.h | 26 +++++++++++
>> include/hw/cxl/cxl_device.h | 6 +++
>> 4 files changed, 144 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index a02d130926..4f25caecea 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -122,6 +122,7 @@ enum {
>> #define MANAGEMENT_COMMAND 0x0
>> MHD = 0x55,
>> #define GET_MHD_INFO 0x0
>> + #define GET_MHD_HEAD_INFO 0x1
>> };
>>
>> /* CCI Message Format CXL r3.1 Figure 7-19 */
>> @@ -267,6 +268,23 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
>> return CXL_MBOX_UNSUPPORTED;
>> }
>>
>> +/*
>> + * CXL r3.2 section 7.6.7.5.2 - Get Multi-Headed Head Info (Opcode 5501h)
>> + */
>> +static CXLRetCode cmd_mhd_get_head_info(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);
>> + CXLType3Class *cvc = CXL_TYPE3_GET_CLASS(ct3d);
>> + if (cvc->mhd_get_head_info) {
>> + return cvc->mhd_get_head_info(cmd, payload_in, len_in, payload_out,
>> + len_out, cci);
>> + }
>> + return CXL_MBOX_UNSUPPORTED;
>> +}
>> +
>> static CXLRetCode cmd_events_get_records(const struct cxl_cmd *cmd,
>> uint8_t *payload_in, size_t len_in,
>> uint8_t *payload_out, size_t *len_out,
>> @@ -3429,6 +3447,7 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
>> "MEDIA_AND_POISON_GET_SCAN_MEDIA_RESULTS",
>> cmd_media_get_scan_media_results, 0, 0 },
>> [MHD][GET_MHD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_info, 2, 0},
>> + [MHD][GET_MHD_HEAD_INFO] = { "GET_MULTI_HEADED_INFO", cmd_mhd_get_head_info, 2, 0},
>The name here "GET_MULTI_HEADED_INFO" is the same as the previous command.
Yes, It should be GET_HEAD_INFO. Will fix in V1 patch.
>> };
>>
>...
>> +
>> +static void cxl_mhsld_init_head_info(CXLMHSLDState *s, PCIDevice *pdev)
>> +{
>> + uint16_t lnksta = 0;
>> + uint16_t current_link_speed = 0;
>> + uint16_t negotiated_link_width = 0;
>> + uint16_t lnkcap = 0, lnkcap2 = 0;
>> + uint16_t max_link_width = 0;
>> + uint16_t max_link_speed = 0;
>> + uint16_t supported_link_speeds_vector = 0;
>> +
>> + lnksta = pdev->config_read(pdev,
>> + pdev->exp.exp_cap + PCI_EXP_LNKSTA,
>> + sizeof(lnksta));
>> + lnkcap = pdev->config_read(pdev,
>> + pdev->exp.exp_cap + PCI_EXP_LNKCAP,
>> + sizeof(lnkcap));
>> + lnkcap2 = pdev->config_read(pdev,
>> + pdev->exp.exp_cap + PCI_EXP_LNKCAP2,
>> + sizeof(lnkcap2));
>> + supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
>> + max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
>> + max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
>> + current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
>> + negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
>> +
>> + s->mhd_state->head_info_blocks[s->mhd_head].port_number = s->mhd_head;
>> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_width = max_link_width;
>> + s->mhd_state->head_info_blocks[s->mhd_head].nego_link_width =
>> + negotiated_link_width;
>> + s->mhd_state->head_info_blocks[s->mhd_head].supp_link_speeds_vector =
>> + supported_link_speeds_vector;
>> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_speed =
>> + max_link_speed;
>> + s->mhd_state->head_info_blocks[s->mhd_head].current_link_speed =
>> + current_link_speed;
>> + s->mhd_state->head_info_blocks[s->mhd_head].ltssm_state = 0x7;
>I think it would be helpful to make a define for the ltssm states?
Sure, Will fix in V1 patch
>> + s->mhd_state->head_info_blocks[s->mhd_head].first_nego_lane_num = 0;
>> + s->mhd_state->head_info_blocks[s->mhd_head].link_state_flags = 0;
>> +}
>> +
>> /* Returns starting index of region in MHD map. */
>> static inline size_t cxl_mhsld_find_dc_region_start(PCIDevice *d,
>> CXLDCRegion *r)
>> @@ -376,7 +465,7 @@ static void cxl_mhsld_realize(PCIDevice *pci_dev, Error **errp)
>> }
>>
>> cxl_mhsld_state_initialize(s, dc_size);
>> -
>> + cxl_mhsld_init_head_info(s, pci_dev);
>> /* Set the LD ownership for this head to this system */
>> s->mhd_state->ldmap[s->mhd_head] = s->mhd_head;
>> return;
>> @@ -428,6 +517,7 @@ static void cxl_mhsld_class_init(ObjectClass *klass, void *data)
>>
>> CXLType3Class *cvc = CXL_TYPE3_CLASS(klass);
>> cvc->mhd_get_info = cmd_mhd_get_info;
>> + cvc->mhd_get_head_info = cmd_mhd_get_head_info;
>> cvc->mhd_access_valid = cxl_mhsld_access_valid;
>> cvc->mhd_reserve_extents = cxl_mhsld_reserve_extents;
>> cvc->mhd_reclaim_extents = cxl_mhsld_reclaim_extents;
>> diff --git a/hw/cxl/mhsld/mhsld.h b/hw/cxl/mhsld/mhsld.h
>> index e7ead1f0d2..c9fbec71ca 100644
>> --- a/hw/cxl/mhsld/mhsld.h
>> +++ b/hw/cxl/mhsld/mhsld.h
>> @@ -23,6 +23,18 @@
>> */
>> #define MHSLD_HEADS (8)
>>
>> +typedef struct MHDHeadInfoBlock {
>> + uint8_t port_number;
>> + uint8_t max_link_width;
>> + uint8_t nego_link_width;
>> + uint8_t supp_link_speeds_vector;
>> + uint8_t max_link_speed;
>> + uint8_t current_link_speed;
>> + uint8_t ltssm_state;
>> + uint8_t first_nego_lane_num;
>> + uint8_t link_state_flags;
>> +} QEMU_PACKED MHDHeadInfoBlock;
>> +
>> /*
>> * The shared state cannot have 2 variable sized regions
>> * so we have to max out the ldmap.
>> @@ -32,6 +44,7 @@ typedef struct MHSLDSharedState {
>> uint8_t nr_lds;
>> uint8_t ldmap[MHSLD_HEADS];
>> uint64_t nr_blocks;
>> + MHDHeadInfoBlock head_info_blocks[MHSLD_HEADS];
>> uint8_t blocks[];
>> } MHSLDSharedState;
>>
>> @@ -52,6 +65,7 @@ struct CXLMHSLDClass {
>> enum {
>> MHSLD_MHD = 0x55,
>> #define GET_MHD_INFO 0x0
>> + #define GET_MHD_HEAD_INFO 0x1
>> };
>>
>> /*
>> @@ -72,4 +86,16 @@ typedef struct MHDGetInfoOutput {
>> uint16_t resv2;
>> uint8_t ldmap[];
>> } QEMU_PACKED MHDGetInfoOutput;
>> +
>> +typedef struct MHDGetHeadInfoInput {
>> + uint8_t start_head;
>> + uint8_t nr_heads;
>> +} QEMU_PACKED MHDGetHeadInfoInput;
>> +
>> +typedef struct MHDGetHeadInfoOutput {
>> + uint8_t nr_heads;
>> + uint8_t rsvd[3];
>> + MHDHeadInfoBlock head_info_list[];
>> +} QEMU_PACKED MHDGetHeadInfoOutput;
>> +
>> #endif
>> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
>> index ca515cab13..c93c71c45d 100644
>> --- a/include/hw/cxl/cxl_device.h
>> +++ b/include/hw/cxl/cxl_device.h
>> @@ -732,6 +732,12 @@ struct CXLType3Class {
>> uint8_t *payload_out,
>> size_t *len_out,
>> CXLCCI *cci);
>> + CXLRetCode (*mhd_get_head_info)(const struct cxl_cmd *cmd,
>> + uint8_t *payload_in,
>> + size_t len_in,
>> + uint8_t *payload_out,
>> + size_t *len_out,
>> + CXLCCI *cci);
>I think the indent here needs to be aligned with the open parentheses on
>the first line of the function declaration
I’ll fix in V1 patch.
>> bool (*mhd_access_valid)(PCIDevice *d, uint64_t addr, unsigned int size);
>> bool (*mhd_reserve_extents)(PCIDevice *d,
>> CxlDynamicCapacityExtentList *records,
>> --
>> 2.34.1
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h)
2025-05-22 6:31 ` [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h) Vinayak Holikatti
@ 2025-05-29 12:13 ` Jonathan Cameron via
2025-05-22 19:15 ` Davidlohr Bueso
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2025-05-29 12:13 UTC (permalink / raw)
To: Vinayak Holikatti
Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g,
krish.reddy, a.manzanares, alok.rathore
On Thu, 22 May 2025 12:01:35 +0530
Vinayak Holikatti <vinayak.kh@samsung.com> wrote:
> CXL spec 3.2 section 7.6.7.5.2 describes Get Head Info.
>
> Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com>
Hi Vinayak,
Some code simplification suggestions below.
> ---
> This patch is generated against Jonathan Cameron's branch cxl-2025-03-20
>
> hw/cxl/cxl-mailbox-utils.c | 21 +++++++++
> hw/cxl/mhsld/mhsld.c | 92 ++++++++++++++++++++++++++++++++++++-
> hw/cxl/mhsld/mhsld.h | 26 +++++++++++
> include/hw/cxl/cxl_device.h | 6 +++
> 4 files changed, 144 insertions(+), 1 deletion(-)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index a02d130926..4f25caecea 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> diff --git a/hw/cxl/mhsld/mhsld.c b/hw/cxl/mhsld/mhsld.c
> index 9f633b3bed..981546b5ff 100644
> --- a/hw/cxl/mhsld/mhsld.c
> +++ b/hw/cxl/mhsld/mhsld.c
> @@ -61,9 +61,57 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
> +/*
> + * CXL r3.2 section 7.6.7.5.2 - Get Head Info (Opcode 5501h)
> + *
> + * This command retrieves the number of heads, number of supported LDs,
> + * and Head-to-LD mapping of a Multi-Headed device.
> + */
> +static CXLRetCode cmd_mhd_get_head_info(const struct cxl_cmd *cmd,
> + uint8_t *payload_in, size_t len_in,
> + uint8_t *payload_out, size_t *len_out,
> + CXLCCI *cci)
> +{
> + CXLMHSLDState *s = CXL_MHSLD(cci->d);
> + MHDGetHeadInfoInput *input = (void *)payload_in;
> + MHDGetHeadInfoOutput *output = (void *)payload_out;
> + int i = 0;
> +
> + if (input->start_head > MHSLD_HEADS) {
> + return CXL_MBOX_INVALID_INPUT;
> + }
> +
> + output->nr_heads = MIN((MHSLD_HEADS - input->start_head), input->nr_heads);
> + for (i = input->start_head; i < input->start_head + output->nr_heads; i++) {
Can we get away with a memcpy()? Any endian issues on any of these?
> + output->head_info_list[i].port_number =
> + s->mhd_state->head_info_blocks[i].port_number;
> + output->head_info_list[i].max_link_width =
> + s->mhd_state->head_info_blocks[i].max_link_width;
> + output->head_info_list[i].nego_link_width =
> + s->mhd_state->head_info_blocks[i].nego_link_width;
> + output->head_info_list[i].supp_link_speeds_vector =
> + s->mhd_state->head_info_blocks[i].supp_link_speeds_vector;
> + output->head_info_list[i].max_link_speed =
> + s->mhd_state->head_info_blocks[i].max_link_speed;
> + output->head_info_list[i].current_link_speed =
> + s->mhd_state->head_info_blocks[i].current_link_speed;
> + output->head_info_list[i].ltssm_state =
> + s->mhd_state->head_info_blocks[i].ltssm_state;
> + output->head_info_list[i].first_nego_lane_num =
> + s->mhd_state->head_info_blocks[i].first_nego_lane_num;
> + output->head_info_list[i].link_state_flags =
> + s->mhd_state->head_info_blocks[i].link_state_flags;
> + }
> +
> + *len_out = sizeof(*output) + output->nr_heads * sizeof(MHDHeadInfoBlock);
> + return CXL_MBOX_SUCCESS;
> +}
> static const Property cxl_mhsld_props[] = {
> @@ -166,6 +214,47 @@ static void cxl_mhsld_state_initialize(CXLMHSLDState *s, size_t dc_size)
> s->mhd_state->nr_blocks = dc_size / MHSLD_BLOCK_SZ;
> }
>
> +
> +static void cxl_mhsld_init_head_info(CXLMHSLDState *s, PCIDevice *pdev)
> +{
> + uint16_t lnksta = 0;
No need to initialize when they are always set below.
> + uint16_t current_link_speed = 0;
> + uint16_t negotiated_link_width = 0;
> + uint16_t lnkcap = 0, lnkcap2 = 0;
> + uint16_t max_link_width = 0;
> + uint16_t max_link_speed = 0;
Once you drop the unnecessary init combine width and speed on one line.
Or as below, get rid of most of these local variables entirely.
> + uint16_t supported_link_speeds_vector = 0;
> +
> + lnksta = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKSTA,
> + sizeof(lnksta));
> + lnkcap = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKCAP,
> + sizeof(lnkcap));
> + lnkcap2 = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKCAP2,
> + sizeof(lnkcap2));
> + supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
Worth considering adding defines for that to incluw/hw/pci/pcie_regs.h
> + max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
Use PCI_EXP_LNK_MLW_SHIFT to extract that.
(we should also tidy this up in physical port state.
> + max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
> + current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
> + negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
Similar - there should be a suitable define for that shift.
> +
> + s->mhd_state->head_info_blocks[s->mhd_head].port_number = s->mhd_head;
I would use something like
s->mhd_state->head_info_blocks[s->mhd_head] = (MHDHeadInfoBlock) {
.max_link_width = lnkcap & PCI_EXP_LNKCAP_SLS,
.nego_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4, //with the define
.supp_link_speeds_vector = (lnkcap2 & 0xFE) >> 1,
etc
};
> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_width = max_link_width;
> + s->mhd_state->head_info_blocks[s->mhd_head].nego_link_width =
> + negotiated_link_width;
> + s->mhd_state->head_info_blocks[s->mhd_head].supp_link_speeds_vector =
> + supported_link_speeds_vector;
> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_speed =
> + max_link_speed;
> + s->mhd_state->head_info_blocks[s->mhd_head].current_link_speed =
> + current_link_speed;
> + s->mhd_state->head_info_blocks[s->mhd_head].ltssm_state = 0x7;
> + s->mhd_state->head_info_blocks[s->mhd_head].first_nego_lane_num = 0;
> + s->mhd_state->head_info_blocks[s->mhd_head].link_state_flags = 0;
> +}
> +
> /* Returns starting index of region in MHD map. */
> static inline size_t cxl_mhsld_find_dc_region_start(PCIDevice *d,
> CXLDCRegion *r)
> @@ -376,7 +465,7 @@ static void cxl_mhsld_realize(PCIDevice *pci_dev, Error **errp)
> }
>
> cxl_mhsld_state_initialize(s, dc_size);
> -
> + cxl_mhsld_init_head_info(s, pci_dev);
Avoid the white space noise by leaving a blank line here.
> /* Set the LD ownership for this head to this system */
> s->mhd_state->ldmap[s->mhd_head] = s->mhd_head;
> return;
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h)
@ 2025-05-29 12:13 ` Jonathan Cameron via
0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron via @ 2025-05-29 12:13 UTC (permalink / raw)
To: Vinayak Holikatti
Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g,
krish.reddy, a.manzanares, alok.rathore
On Thu, 22 May 2025 12:01:35 +0530
Vinayak Holikatti <vinayak.kh@samsung.com> wrote:
> CXL spec 3.2 section 7.6.7.5.2 describes Get Head Info.
>
> Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com>
Hi Vinayak,
Some code simplification suggestions below.
> ---
> This patch is generated against Jonathan Cameron's branch cxl-2025-03-20
>
> hw/cxl/cxl-mailbox-utils.c | 21 +++++++++
> hw/cxl/mhsld/mhsld.c | 92 ++++++++++++++++++++++++++++++++++++-
> hw/cxl/mhsld/mhsld.h | 26 +++++++++++
> include/hw/cxl/cxl_device.h | 6 +++
> 4 files changed, 144 insertions(+), 1 deletion(-)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index a02d130926..4f25caecea 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> diff --git a/hw/cxl/mhsld/mhsld.c b/hw/cxl/mhsld/mhsld.c
> index 9f633b3bed..981546b5ff 100644
> --- a/hw/cxl/mhsld/mhsld.c
> +++ b/hw/cxl/mhsld/mhsld.c
> @@ -61,9 +61,57 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
> +/*
> + * CXL r3.2 section 7.6.7.5.2 - Get Head Info (Opcode 5501h)
> + *
> + * This command retrieves the number of heads, number of supported LDs,
> + * and Head-to-LD mapping of a Multi-Headed device.
> + */
> +static CXLRetCode cmd_mhd_get_head_info(const struct cxl_cmd *cmd,
> + uint8_t *payload_in, size_t len_in,
> + uint8_t *payload_out, size_t *len_out,
> + CXLCCI *cci)
> +{
> + CXLMHSLDState *s = CXL_MHSLD(cci->d);
> + MHDGetHeadInfoInput *input = (void *)payload_in;
> + MHDGetHeadInfoOutput *output = (void *)payload_out;
> + int i = 0;
> +
> + if (input->start_head > MHSLD_HEADS) {
> + return CXL_MBOX_INVALID_INPUT;
> + }
> +
> + output->nr_heads = MIN((MHSLD_HEADS - input->start_head), input->nr_heads);
> + for (i = input->start_head; i < input->start_head + output->nr_heads; i++) {
Can we get away with a memcpy()? Any endian issues on any of these?
> + output->head_info_list[i].port_number =
> + s->mhd_state->head_info_blocks[i].port_number;
> + output->head_info_list[i].max_link_width =
> + s->mhd_state->head_info_blocks[i].max_link_width;
> + output->head_info_list[i].nego_link_width =
> + s->mhd_state->head_info_blocks[i].nego_link_width;
> + output->head_info_list[i].supp_link_speeds_vector =
> + s->mhd_state->head_info_blocks[i].supp_link_speeds_vector;
> + output->head_info_list[i].max_link_speed =
> + s->mhd_state->head_info_blocks[i].max_link_speed;
> + output->head_info_list[i].current_link_speed =
> + s->mhd_state->head_info_blocks[i].current_link_speed;
> + output->head_info_list[i].ltssm_state =
> + s->mhd_state->head_info_blocks[i].ltssm_state;
> + output->head_info_list[i].first_nego_lane_num =
> + s->mhd_state->head_info_blocks[i].first_nego_lane_num;
> + output->head_info_list[i].link_state_flags =
> + s->mhd_state->head_info_blocks[i].link_state_flags;
> + }
> +
> + *len_out = sizeof(*output) + output->nr_heads * sizeof(MHDHeadInfoBlock);
> + return CXL_MBOX_SUCCESS;
> +}
> static const Property cxl_mhsld_props[] = {
> @@ -166,6 +214,47 @@ static void cxl_mhsld_state_initialize(CXLMHSLDState *s, size_t dc_size)
> s->mhd_state->nr_blocks = dc_size / MHSLD_BLOCK_SZ;
> }
>
> +
> +static void cxl_mhsld_init_head_info(CXLMHSLDState *s, PCIDevice *pdev)
> +{
> + uint16_t lnksta = 0;
No need to initialize when they are always set below.
> + uint16_t current_link_speed = 0;
> + uint16_t negotiated_link_width = 0;
> + uint16_t lnkcap = 0, lnkcap2 = 0;
> + uint16_t max_link_width = 0;
> + uint16_t max_link_speed = 0;
Once you drop the unnecessary init combine width and speed on one line.
Or as below, get rid of most of these local variables entirely.
> + uint16_t supported_link_speeds_vector = 0;
> +
> + lnksta = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKSTA,
> + sizeof(lnksta));
> + lnkcap = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKCAP,
> + sizeof(lnkcap));
> + lnkcap2 = pdev->config_read(pdev,
> + pdev->exp.exp_cap + PCI_EXP_LNKCAP2,
> + sizeof(lnkcap2));
> + supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
Worth considering adding defines for that to incluw/hw/pci/pcie_regs.h
> + max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
Use PCI_EXP_LNK_MLW_SHIFT to extract that.
(we should also tidy this up in physical port state.
> + max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
> + current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
> + negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
Similar - there should be a suitable define for that shift.
> +
> + s->mhd_state->head_info_blocks[s->mhd_head].port_number = s->mhd_head;
I would use something like
s->mhd_state->head_info_blocks[s->mhd_head] = (MHDHeadInfoBlock) {
.max_link_width = lnkcap & PCI_EXP_LNKCAP_SLS,
.nego_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4, //with the define
.supp_link_speeds_vector = (lnkcap2 & 0xFE) >> 1,
etc
};
> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_width = max_link_width;
> + s->mhd_state->head_info_blocks[s->mhd_head].nego_link_width =
> + negotiated_link_width;
> + s->mhd_state->head_info_blocks[s->mhd_head].supp_link_speeds_vector =
> + supported_link_speeds_vector;
> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_speed =
> + max_link_speed;
> + s->mhd_state->head_info_blocks[s->mhd_head].current_link_speed =
> + current_link_speed;
> + s->mhd_state->head_info_blocks[s->mhd_head].ltssm_state = 0x7;
> + s->mhd_state->head_info_blocks[s->mhd_head].first_nego_lane_num = 0;
> + s->mhd_state->head_info_blocks[s->mhd_head].link_state_flags = 0;
> +}
> +
> /* Returns starting index of region in MHD map. */
> static inline size_t cxl_mhsld_find_dc_region_start(PCIDevice *d,
> CXLDCRegion *r)
> @@ -376,7 +465,7 @@ static void cxl_mhsld_realize(PCIDevice *pci_dev, Error **errp)
> }
>
> cxl_mhsld_state_initialize(s, dc_size);
> -
> + cxl_mhsld_init_head_info(s, pci_dev);
Avoid the white space noise by leaving a blank line here.
> /* Set the LD ownership for this head to this system */
> s->mhd_state->ldmap[s->mhd_head] = s->mhd_head;
> return;
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] Add support for FMAPI Get Mutliheaded Head info opcode (5501h)
2025-05-29 12:13 ` Jonathan Cameron via
(?)
@ 2025-07-01 13:19 ` Alok Rathore
-1 siblings, 0 replies; 10+ messages in thread
From: Alok Rathore @ 2025-07-01 13:19 UTC (permalink / raw)
To: Jonathan Cameron
Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g,
krish.reddy, a.manzanares, alokrathore20
[-- Attachment #1: Type: text/plain, Size: 7636 bytes --]
On 29/05/25 01:13PM, Jonathan Cameron wrote:
>On Thu, 22 May 2025 12:01:35 +0530
>Vinayak Holikatti <vinayak.kh@samsung.com> wrote:
>
>> CXL spec 3.2 section 7.6.7.5.2 describes Get Head Info.
>>
>> Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com>
>
>Hi Vinayak,
>
>Some code simplification suggestions below.
Thanks Jonathan for reviewing the patch.
Alok
>
>> ---
>> This patch is generated against Jonathan Cameron's branch cxl-2025-03-20
>>
>> hw/cxl/cxl-mailbox-utils.c | 21 +++++++++
>> hw/cxl/mhsld/mhsld.c | 92 ++++++++++++++++++++++++++++++++++++-
>> hw/cxl/mhsld/mhsld.h | 26 +++++++++++
>> include/hw/cxl/cxl_device.h | 6 +++
>> 4 files changed, 144 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index a02d130926..4f25caecea 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>
>> diff --git a/hw/cxl/mhsld/mhsld.c b/hw/cxl/mhsld/mhsld.c
>> index 9f633b3bed..981546b5ff 100644
>> --- a/hw/cxl/mhsld/mhsld.c
>> +++ b/hw/cxl/mhsld/mhsld.c
>> @@ -61,9 +61,57 @@ static CXLRetCode cmd_mhd_get_info(const struct cxl_cmd *cmd,
>> return CXL_MBOX_SUCCESS;
>> }
>>
>> +/*
>> + * CXL r3.2 section 7.6.7.5.2 - Get Head Info (Opcode 5501h)
>> + *
>> + * This command retrieves the number of heads, number of supported LDs,
>> + * and Head-to-LD mapping of a Multi-Headed device.
>> + */
>> +static CXLRetCode cmd_mhd_get_head_info(const struct cxl_cmd *cmd,
>> + uint8_t *payload_in, size_t len_in,
>> + uint8_t *payload_out, size_t *len_out,
>> + CXLCCI *cci)
>> +{
>> + CXLMHSLDState *s = CXL_MHSLD(cci->d);
>> + MHDGetHeadInfoInput *input = (void *)payload_in;
>> + MHDGetHeadInfoOutput *output = (void *)payload_out;
>> + int i = 0;
>> +
>> + if (input->start_head > MHSLD_HEADS) {
>> + return CXL_MBOX_INVALID_INPUT;
>> + }
>> +
>> + output->nr_heads = MIN((MHSLD_HEADS - input->start_head), input->nr_heads);
>> + for (i = input->start_head; i < input->start_head + output->nr_heads; i++) {
>
>Can we get away with a memcpy()? Any endian issues on any of these?
Sure, I’ll check with memcpy()
>
>> + output->head_info_list[i].port_number =
>> + s->mhd_state->head_info_blocks[i].port_number;
>> + output->head_info_list[i].max_link_width =
>> + s->mhd_state->head_info_blocks[i].max_link_width;
>> + output->head_info_list[i].nego_link_width =
>> + s->mhd_state->head_info_blocks[i].nego_link_width;
>> + output->head_info_list[i].supp_link_speeds_vector =
>> + s->mhd_state->head_info_blocks[i].supp_link_speeds_vector;
>> + output->head_info_list[i].max_link_speed =
>> + s->mhd_state->head_info_blocks[i].max_link_speed;
>> + output->head_info_list[i].current_link_speed =
>> + s->mhd_state->head_info_blocks[i].current_link_speed;
>> + output->head_info_list[i].ltssm_state =
>> + s->mhd_state->head_info_blocks[i].ltssm_state;
>> + output->head_info_list[i].first_nego_lane_num =
>> + s->mhd_state->head_info_blocks[i].first_nego_lane_num;
>> + output->head_info_list[i].link_state_flags =
>> + s->mhd_state->head_info_blocks[i].link_state_flags;
>> + }
>> +
>> + *len_out = sizeof(*output) + output->nr_heads * sizeof(MHDHeadInfoBlock);
>> + return CXL_MBOX_SUCCESS;
>> +}
>
>> static const Property cxl_mhsld_props[] = {
>> @@ -166,6 +214,47 @@ static void cxl_mhsld_state_initialize(CXLMHSLDState *s, size_t dc_size)
>> s->mhd_state->nr_blocks = dc_size / MHSLD_BLOCK_SZ;
>> }
>>
>> +
>> +static void cxl_mhsld_init_head_info(CXLMHSLDState *s, PCIDevice *pdev)
>> +{
>> + uint16_t lnksta = 0;
>
>No need to initialize when they are always set below.
Sure, I’ll fix in V1 patch.
>
>> + uint16_t current_link_speed = 0;
>> + uint16_t negotiated_link_width = 0;
>> + uint16_t lnkcap = 0, lnkcap2 = 0;
>> + uint16_t max_link_width = 0;
>> + uint16_t max_link_speed = 0;
>
>Once you drop the unnecessary init combine width and speed on one line.
>Or as below, get rid of most of these local variables entirely.
>
>> + uint16_t supported_link_speeds_vector = 0;
>> +
>> + lnksta = pdev->config_read(pdev,
>> + pdev->exp.exp_cap + PCI_EXP_LNKSTA,
>> + sizeof(lnksta));
>> + lnkcap = pdev->config_read(pdev,
>> + pdev->exp.exp_cap + PCI_EXP_LNKCAP,
>> + sizeof(lnkcap));
>> + lnkcap2 = pdev->config_read(pdev,
>> + pdev->exp.exp_cap + PCI_EXP_LNKCAP2,
>> + sizeof(lnkcap2));
>> + supported_link_speeds_vector = (lnkcap2 & 0xFE) >> 1;
>Worth considering adding defines for that to incluw/hw/pci/pcie_regs.h
Okay. I’ll add in V1 patch.
>
>> + max_link_width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
>
>Use PCI_EXP_LNK_MLW_SHIFT to extract that.
>(we should also tidy this up in physical port state.
Sure. I’ll fix in V1 patch.
>
>
>> + max_link_speed = lnkcap & PCI_EXP_LNKCAP_SLS;
>> + current_link_speed = lnksta & PCI_EXP_LNKSTA_CLS;
>> + negotiated_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4;
>Similar - there should be a suitable define for that shift.
>> +
>> + s->mhd_state->head_info_blocks[s->mhd_head].port_number = s->mhd_head;
>
>I would use something like
>
> s->mhd_state->head_info_blocks[s->mhd_head] = (MHDHeadInfoBlock) {
> .max_link_width = lnkcap & PCI_EXP_LNKCAP_SLS,
> .nego_link_width = (lnksta & PCI_EXP_LNKSTA_NLW) >> 4, //with the define
> .supp_link_speeds_vector = (lnkcap2 & 0xFE) >> 1,
>etc
> };
Sure, I’ll remove local variable and assign in this way.
>> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_width = max_link_width;
>> + s->mhd_state->head_info_blocks[s->mhd_head].nego_link_width =
>> + negotiated_link_width;
>> + s->mhd_state->head_info_blocks[s->mhd_head].supp_link_speeds_vector =
>> + supported_link_speeds_vector;
>> + s->mhd_state->head_info_blocks[s->mhd_head].max_link_speed =
>> + max_link_speed;
>> + s->mhd_state->head_info_blocks[s->mhd_head].current_link_speed =
>> + current_link_speed;
>> + s->mhd_state->head_info_blocks[s->mhd_head].ltssm_state = 0x7;
>> + s->mhd_state->head_info_blocks[s->mhd_head].first_nego_lane_num = 0;
>> + s->mhd_state->head_info_blocks[s->mhd_head].link_state_flags = 0;
>> +}
>> +
>> /* Returns starting index of region in MHD map. */
>> static inline size_t cxl_mhsld_find_dc_region_start(PCIDevice *d,
>> CXLDCRegion *r)
>> @@ -376,7 +465,7 @@ static void cxl_mhsld_realize(PCIDevice *pci_dev, Error **errp)
>> }
>>
>> cxl_mhsld_state_initialize(s, dc_size);
>> -
>> + cxl_mhsld_init_head_info(s, pci_dev);
>Avoid the white space noise by leaving a blank line here.
Okay, Will fix in V1 patch
>
>> /* Set the LD ownership for this head to this system */
>> s->mhd_state->ldmap[s->mhd_head] = s->mhd_head;
>> return;
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread