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 0EDE6C001DB for ; Mon, 7 Aug 2023 11:03:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231408AbjHGLDo (ORCPT ); Mon, 7 Aug 2023 07:03:44 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41158 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229786AbjHGLDa (ORCPT ); Mon, 7 Aug 2023 07:03:30 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E533F1735 for ; Mon, 7 Aug 2023 04:02:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1691406136; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=4tVoX28GTGtCUXdSQaBJtaWNWRKg0UVBLfIJI4oLwi0=; b=Ov+++bUkj5yfbWNk8gHzYpx3K5cU6xlyE2nBtWFqjGYrovui20x8EKnNlKynwiN+Zg7kMM xTOeZwAQXz+ZWPejU3qlzP5IetL4D1b51+ZFmQX5VpCqyXacBRHTzRm/RlT14MQkDhr2T7 IEnSu4A3HruUI8C2Es9IE2TzUNft2Ok= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-173-uZHJVsXfOl-XqYPNakK6oA-1; Mon, 07 Aug 2023 07:02:14 -0400 X-MC-Unique: uZHJVsXfOl-XqYPNakK6oA-1 Received: by mail-ed1-f71.google.com with SMTP id 4fb4d7f45d1cf-5218b9647a8so2922497a12.1 for ; Mon, 07 Aug 2023 04:02:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691406133; x=1692010933; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=4tVoX28GTGtCUXdSQaBJtaWNWRKg0UVBLfIJI4oLwi0=; b=BIh21nNa+WXCXUc0mf6tOWlWdpSITs9qJu8RRbHC2ViHl3ALwFwf1BmC1Xoh1qwRVf zdgPOcGZSHTzfAJMihWL430Qtxs9UC7VyvkNhmgFJsNKlQFnfJ7ErY6rdQ8Ojod4c5SJ 7rqIx8XK7fzHWbeqOCGztUh6uVjCGj3BpwDUWNSiF6wBX/OE9I6auNqLEJ4y4bOoyvdx yfEf7ySlQVvfqNC9QOQPYdr/puqG3Bm4xLj6UmGo+976REvVGjxUWE11w2Vv+USiPKYs WciXNtJoNaD+AjytXRtE/V868gPyUEtws5psGYB+7C6UhWiQ5jeqOYLrj+uCVlBhKSgi wP9g== X-Gm-Message-State: AOJu0Yyp/qimF3EqVZomncFDtQqrNR4nJ8jEa5KAyOMoLSXZSeYupMd3 lhFUTHbkehRqVpvFvPEIDdAFfku4WcSDPpszv8tNvVOIjTz8oJ8uTmgTw6MjZOdbanpO0DeBK9N 58qbb+Wdy++tFhumTQXzO7dM= X-Received: by 2002:aa7:da91:0:b0:523:f29:a912 with SMTP id q17-20020aa7da91000000b005230f29a912mr5825459eds.21.1691406133618; Mon, 07 Aug 2023 04:02:13 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEpV1CiOzHk73Q0S1VjzXSs0/DvHhdz0S3B8+70wxb75oVf2/18bmu8foLRZEbGwYEDxL6b1A== X-Received: by 2002:aa7:da91:0:b0:523:f29:a912 with SMTP id q17-20020aa7da91000000b005230f29a912mr5825448eds.21.1691406133282; Mon, 07 Aug 2023 04:02:13 -0700 (PDT) Received: from [10.40.98.142] ([78.108.130.194]) by smtp.gmail.com with ESMTPSA id t10-20020aa7d4ca000000b005226f281bc5sm5036828edr.25.2023.08.07.04.02.11 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 07 Aug 2023 04:02:12 -0700 (PDT) Message-ID: <145d7375-0e58-b7cf-6240-5d8bc16b0344@redhat.com> Date: Mon, 7 Aug 2023 13:02:11 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Subject: Re: [PATCH net-next v2 1/5] platform/x86: intel_pmc_core: Add IPC mailbox accessor function and add SoC register access Content-Language: en-US To: Choong Yong Liang , Rajneesh Bhardwaj , David E Box , Mark Gross , Jose Abreu , Andrew Lunn , Heiner Kallweit , Russell King , "David S . Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , =?UTF-8?Q?Marek_Beh=c3=ban?= , Jean Delvare , Guenter Roeck , Giuseppe Cavallaro , Alexandre Torgue , Jose Abreu , Maxime Coquelin , Richard Cochran , Philipp Zabel , Alexei Starovoitov , Daniel Borkmann , Jesper Dangaard Brouer , John Fastabend , Wong Vee Khee , Jon Hunter , Jesse Brandeburg , Shenwei Wang , Andrey Konovalov , Jochen Henneberg Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, platform-driver-x86@vger.kernel.org, linux-hwmon@vger.kernel.org, bpf@vger.kernel.org, Voon Wei Feng , Tan Tee Min , Michael Sit Wei Hong , Lai Peter Jun Ann References: <20230804084527.2082302-1-yong.liang.choong@linux.intel.com> <20230804084527.2082302-2-yong.liang.choong@linux.intel.com> From: Hans de Goede In-Reply-To: <20230804084527.2082302-2-yong.liang.choong@linux.intel.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-hwmon@vger.kernel.org Hi David, On 8/4/23 10:45, Choong Yong Liang wrote: > From: "David E. Box" > > - Exports intel_pmc_core_ipc() for host access to the PMC IPC mailbox > - Add support to use IPC command allows host to access SoC registers > through PMC firmware that are otherwise inaccessible to the host due to > security policies. > > Signed-off-by: David E. Box > Signed-off-by: Chao Qin > Signed-off-by: Choong Yong Liang The new exported intel_pmc_core_ipc() function does not seem to depend on any existing PMC code. IMHO it would be better to put this in a new .c file under arch/x86/platform/intel/ this is where similar helpers like the iosf_mbi functions also live. This also avoids Kconfig complications. Currently the drivers/platform/x86/intel/pmc/core.c code is only build if CONFIG_X86_PLATFORM_DEVICES and CONFIG_INTEL_PMC_CORE are both set. So if a driver wants to make sure this is enabled by selecting them then it needs to select both. Talking about Kconfig: #if IS_ENABLED(CONFIG_INTEL_PMC_CORE) int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf); #else static inline int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf) { return -ENODEV; } #endif /* CONFIG_INTEL_PMC_CORE */ Notice that CONFIG_INTEL_PMC_CORE is a tristate, so pmc might be build as a module where as a consumer of intel_pmc_core_ipc() might end up builtin in which case this will not work without extra Kconfig protection. And if you are going to add extra Kconfig you might just as well select or depend on INTEL_PMC_CORE and drop the #if . Regards, Hans > --- > MAINTAINERS | 1 + > drivers/platform/x86/intel/pmc/core.c | 60 +++++++++++++++++++ > .../linux/platform_data/x86/intel_pmc_core.h | 41 +++++++++++++ > 3 files changed, 102 insertions(+) > create mode 100644 include/linux/platform_data/x86/intel_pmc_core.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index 069e176d607a..8a034dee9da9 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10648,6 +10648,7 @@ L: platform-driver-x86@vger.kernel.org > S: Maintained > F: Documentation/ABI/testing/sysfs-platform-intel-pmc > F: drivers/platform/x86/intel/pmc/ > +F: linux/platform_data/x86/intel_pmc_core.h > > INTEL PMIC GPIO DRIVERS > M: Andy Shevchenko > diff --git a/drivers/platform/x86/intel/pmc/core.c b/drivers/platform/x86/intel/pmc/core.c > index 5a36b3f77bc5..6fb1b0f453d8 100644 > --- a/drivers/platform/x86/intel/pmc/core.c > +++ b/drivers/platform/x86/intel/pmc/core.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -28,6 +29,8 @@ > > #include "core.h" > > +#define PMC_IPCS_PARAM_COUNT 7 > + > /* Maximum number of modes supported by platfoms that has low power mode capability */ > const char *pmc_lpm_modes[] = { > "S0i2.0", > @@ -53,6 +56,63 @@ const struct pmc_bit_map msr_map[] = { > {} > }; > > +int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf) > +{ > + struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > + union acpi_object params[PMC_IPCS_PARAM_COUNT] = { > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + {.type = ACPI_TYPE_INTEGER,}, > + }; > + struct acpi_object_list arg_list = { PMC_IPCS_PARAM_COUNT, params }; > + union acpi_object *obj; > + int status; > + > + if (!ipc_cmd || !rbuf) > + return -EINVAL; > + > + /* > + * 0: IPC Command > + * 1: IPC Sub Command > + * 2: Size > + * 3-6: Write Buffer for offset > + */ > + params[0].integer.value = ipc_cmd->cmd; > + params[1].integer.value = ipc_cmd->sub_cmd; > + params[2].integer.value = ipc_cmd->size; > + params[3].integer.value = ipc_cmd->wbuf[0]; > + params[4].integer.value = ipc_cmd->wbuf[1]; > + params[5].integer.value = ipc_cmd->wbuf[2]; > + params[6].integer.value = ipc_cmd->wbuf[3]; > + > + status = acpi_evaluate_object(NULL, "\\IPCS", &arg_list, &buffer); > + if (ACPI_FAILURE(status)) > + return -ENODEV; > + > + obj = buffer.pointer; > + /* Check if the number of elements in package is 5 */ > + if (obj && obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 5) { > + const union acpi_object *objs = obj->package.elements; > + > + if ((u8)objs[0].integer.value != 0) > + return -EINVAL; > + > + rbuf[0] = objs[1].integer.value; > + rbuf[1] = objs[2].integer.value; > + rbuf[2] = objs[3].integer.value; > + rbuf[3] = objs[4].integer.value; > + } else { > + return -EINVAL; > + } > + > + return 0; > +} > +EXPORT_SYMBOL(intel_pmc_core_ipc); > + > static inline u32 pmc_core_reg_read(struct pmc *pmc, int reg_offset) > { > return readl(pmc->regbase + reg_offset); > diff --git a/include/linux/platform_data/x86/intel_pmc_core.h b/include/linux/platform_data/x86/intel_pmc_core.h > new file mode 100644 > index 000000000000..9bb3394fedcf > --- /dev/null > +++ b/include/linux/platform_data/x86/intel_pmc_core.h > @@ -0,0 +1,41 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Intel Core SoC Power Management Controller Header File > + * > + * Copyright (c) 2023, Intel Corporation. > + * All Rights Reserved. > + * > + * Authors: Choong Yong Liang > + * David E. Box > + */ > +#ifndef INTEL_PMC_CORE_H > +#define INTEL_PMC_CORE_H > + > +#define IPC_SOC_REGISTER_ACCESS 0xAA > +#define IPC_SOC_SUB_CMD_READ 0x00 > +#define IPC_SOC_SUB_CMD_WRITE 0x01 > + > +struct pmc_ipc_cmd { > + u32 cmd; > + u32 sub_cmd; > + u32 size; > + u32 wbuf[4]; > +}; > + > +#if IS_ENABLED(CONFIG_INTEL_PMC_CORE) > +/** > + * intel_pmc_core_ipc() - PMC IPC Mailbox accessor > + * @ipc_cmd: struct pmc_ipc_cmd prepared with input to send > + * @rbuf: Allocated u32[4] array for returned IPC data > + * > + * Return: 0 on success. Non-zero on mailbox error > + */ > +int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf); > +#else > +static inline int intel_pmc_core_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf) > +{ > + return -ENODEV; > +} > +#endif /* CONFIG_INTEL_PMC_CORE */ > + > +#endif /* INTEL_PMC_CORE_H */