From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8CDBEC433EF for ; Fri, 4 Mar 2022 13:16:54 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231759AbiCDNRk convert rfc822-to-8bit (ORCPT ); Fri, 4 Mar 2022 08:17:40 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41222 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231496AbiCDNRf (ORCPT ); Fri, 4 Mar 2022 08:17:35 -0500 Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B4D851B50D9 for ; Fri, 4 Mar 2022 05:16:46 -0800 (PST) Received: from fraeml706-chm.china.huawei.com (unknown [172.18.147.206]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4K97dD29hJz67PfY; Fri, 4 Mar 2022 21:15:28 +0800 (CST) Received: from lhreml710-chm.china.huawei.com (10.201.108.61) by fraeml706-chm.china.huawei.com (10.206.15.55) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2308.21; Fri, 4 Mar 2022 14:16:43 +0100 Received: from localhost (10.122.247.231) by lhreml710-chm.china.huawei.com (10.201.108.61) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2308.21; Fri, 4 Mar 2022 13:16:43 +0000 Date: Fri, 4 Mar 2022 13:16:42 +0000 From: Jonathan Cameron To: Alex =?ISO-8859-1?Q?Benn=E9e?= CC: , Marcel Apfelbaum , "Michael S . Tsirkin" , Igor Mammedov , , Ben Widawsky , "Peter Maydell" , , "Shameerali Kolothum Thodi" , Philippe =?ISO-8859-1?Q?Mathieu-Daud=E9?= , Saransh Gupta1 , Shreyas Shah , Chris Browy , Samarth Saxena , "Dan Williams" Subject: Re: [PATCH v6 20/43] hw/cxl/device: Add some trivial commands Message-ID: <20220304131642.000059b9@huawei.com> In-Reply-To: <875yoxld3j.fsf@linaro.org> References: <20220211120747.3074-1-Jonathan.Cameron@huawei.com> <20220211120747.3074-21-Jonathan.Cameron@huawei.com> <875yoxld3j.fsf@linaro.org> Organization: Huawei Technologies R&D (UK) Ltd. X-Mailer: Claws Mail 4.0.0 (GTK+ 3.24.29; x86_64-w64-mingw32) MIME-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 8BIT X-Originating-IP: [10.122.247.231] X-ClientProxiedBy: lhreml706-chm.china.huawei.com (10.201.108.55) To lhreml710-chm.china.huawei.com (10.201.108.61) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-cxl@vger.kernel.org On Tue, 01 Mar 2022 18:46:30 +0000 Alex Bennée wrote: > Jonathan Cameron writes: > > > From: Ben Widawsky > > > > GET_FW_INFO and GET_PARTITION_INFO, for this emulation, is equivalent to > > info already returned in the IDENTIFY command. To have a more robust > > implementation, add those. > > > > Signed-off-by: Ben Widawsky > > Signed-off-by: Jonathan Cameron Hi Alex, > > --- > > hw/cxl/cxl-mailbox-utils.c | 69 +++++++++++++++++++++++++++++++++++++- > > 1 file changed, 68 insertions(+), 1 deletion(-) > > > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > > index 808faec114..d022711b2a 100644 > > --- a/hw/cxl/cxl-mailbox-utils.c > > +++ b/hw/cxl/cxl-mailbox-utils.c > > @@ -44,6 +44,8 @@ enum { > > #define CLEAR_RECORDS 0x1 > > #define GET_INTERRUPT_POLICY 0x2 > > #define SET_INTERRUPT_POLICY 0x3 > > + FIRMWARE_UPDATE = 0x02, > > + #define GET_INFO 0x0 > > TIMESTAMP = 0x03, > > #define GET 0x0 > > #define SET 0x1 > > @@ -52,6 +54,8 @@ enum { > > #define GET_LOG 0x1 > > IDENTIFY = 0x40, > > #define MEMORY_DEVICE 0x0 > > + CCLS = 0x41, > > + #define GET_PARTITION_INFO 0x0 > > }; > > > > /* 8.2.8.4.5.1 Command Return Codes */ > > @@ -114,6 +118,39 @@ DEFINE_MAILBOX_HANDLER_NOP(events_clear_records); > > DEFINE_MAILBOX_HANDLER_ZEROED(events_get_interrupt_policy, 4); > > DEFINE_MAILBOX_HANDLER_NOP(events_set_interrupt_policy); > > > > +/* 8.2.9.2.1 */ > > +static ret_code cmd_firmware_update_get_info(struct cxl_cmd *cmd, > > + CXLDeviceState *cxl_dstate, > > + uint16_t *len) > > +{ > > + struct { > > + uint8_t slots_supported; > > + uint8_t slot_info; > > + uint8_t caps; > > + uint8_t rsvd[0xd]; > > + char fw_rev1[0x10]; > > + char fw_rev2[0x10]; > > + char fw_rev3[0x10]; > > + char fw_rev4[0x10]; > > + } __attribute__((packed)) *fw_info; > > + _Static_assert(sizeof(*fw_info) == 0x50, "Bad firmware info > > size"); > > note: we have QEMU_PACKED, QEMU_BUILD_BUG_ON and friends in compiler.h which are > preferred for potential compiler portability reasons. Replaced all instances of alignment, packed and Static_assert in the patch set with the compiler.h equivalents. Not that has minor affect on some earlier patch sets but feels trivial enough that I've kept RBs etc. > > > + > > + if (cxl_dstate->pmem_size < (256 << 20)) { > > + return CXL_MBOX_INTERNAL_ERROR; > > + } > > + > > + fw_info = (void *)cmd->payload; > > + memset(fw_info, 0, sizeof(*fw_info)); > > + > > + fw_info->slots_supported = 2; > > + fw_info->slot_info = BIT(0) | BIT(3); > > + fw_info->caps = 0; > > + snprintf(fw_info->fw_rev1, 0x10, "BWFW VERSION %02d", 0); > > Given you have a fixed string here could you not: > > pstrcpy(fw_info->fw_rev1, 0x10, "BWFW VERSION 0"); Make sense. > > > + > > + *len = sizeof(*fw_info); > > + return CXL_MBOX_SUCCESS; > > +} > > + > > /* 8.2.9.3.1 */ > > static ret_code cmd_timestamp_get(struct cxl_cmd *cmd, > > CXLDeviceState *cxl_dstate, > > @@ -260,6 +297,33 @@ static ret_code cmd_identify_memory_device(struct cxl_cmd *cmd, > > return CXL_MBOX_SUCCESS; > > } > > > > +static ret_code cmd_ccls_get_partition_info(struct cxl_cmd *cmd, > > + CXLDeviceState *cxl_dstate, > > + uint16_t *len) > > +{ > > + struct { > > + uint64_t active_vmem; > > + uint64_t active_pmem; > > + uint64_t next_vmem; > > + uint64_t next_pmem; > > + } __attribute__((packed)) *part_info = (void *)cmd->payload; > > + _Static_assert(sizeof(*part_info) == 0x20, "Bad get partition info size"); > > + uint64_t size = cxl_dstate->pmem_size; > > + > > + if (!QEMU_IS_ALIGNED(size, 256 << 20)) { > > + return CXL_MBOX_INTERNAL_ERROR; > > + } > > + > > + /* PMEM only */ > > + part_info->active_vmem = 0; > > + part_info->next_vmem = 0; > > + part_info->active_pmem = size / (256 << 20); > > + part_info->next_pmem = part_info->active_pmem; > > + > > + *len = sizeof(*part_info); > > + return CXL_MBOX_SUCCESS; > > +} > > + > > #define IMMEDIATE_CONFIG_CHANGE (1 << 1) > > #define IMMEDIATE_POLICY_CHANGE (1 << 3) > > #define IMMEDIATE_LOG_CHANGE (1 << 4) > > @@ -273,15 +337,18 @@ static struct cxl_cmd cxl_cmd_set[256][256] = { > > cmd_events_get_interrupt_policy, 0, 0 }, > > [EVENTS][SET_INTERRUPT_POLICY] = { "EVENTS_SET_INTERRUPT_POLICY", > > cmd_events_set_interrupt_policy, 4, IMMEDIATE_CONFIG_CHANGE }, > > + [FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO", > > + cmd_firmware_update_get_info, 0, 0 }, > > [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 }, > > [TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set, 8, IMMEDIATE_POLICY_CHANGE }, > > [LOGS][GET_SUPPORTED] = { "LOGS_GET_SUPPORTED", cmd_logs_get_supported, 0, 0 }, > > [LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 }, > > [IDENTIFY][MEMORY_DEVICE] = { "IDENTIFY_MEMORY_DEVICE", > > cmd_identify_memory_device, 0, 0 }, > > + [CCLS][GET_PARTITION_INFO] = { "CCLS_GET_PARTITION_INFO", > > + cmd_ccls_get_partition_info, 0, 0 }, > > }; > > > > - Also fixed this bit of rebasing mess up. Thanks, Jonathan > > void cxl_process_mailbox(CXLDeviceState *cxl_dstate) > > { > > uint16_t ret = CXL_MBOX_SUCCESS; > >