[AMD Official Use Only - General]
>
>
> On Wed, May 24, 2023 at 11:46:13PM +0530, Abhijit Gangurde wrote:
> > MCDI_LOGGING is too generic considering other MCDI users
> > SFC_MCDI_LOGGING and SFC_SIENA_MCDI_LOGGING. Rename it to
> > CDX_MCDI_LOGGING makes it more domain specific.
> >
> > Also, Move CONFIG_CDX_MCDI_LOGGING to header file
> > and make logging variable as a configurable sysfs parameter.
>
> No, sorry, that is not allowed. Just use the normal dynamic debugging
> in the kernel like everyone else uses.
>
> Also you didn't document your new sysfs file, which would have not
> allowed me to take this no matter what.
I'll drop the sysfs changes and send v3 patch for renaming of the macro
so it can be on the rc.
Also, will look onto it to be supported by dynamic debugging.
Thanks,
Abhijit
@@ -18,7 +18,7 @@ config CDX_CONTROLLER
If unsure, say N.
-config MCDI_LOGGING
+config CDX_MCDI_LOGGING
bool "MCDI Logging for the CDX controller"
depends on CDX_CONTROLLER
help
@@ -124,6 +124,23 @@ static struct cdx_ops cdx_ops = {
.dev_configure = cdx_configure_device,
};
+static ssize_t mcdi_logging_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct cdx_controller *cdx = dev_get_drvdata(dev);
+ bool enable;
+
+ if (kstrtobool(buf, &enable) < 0)
+ return -EINVAL;
+
+ if (cdx_enable_mcdi_logging(cdx->priv, enable))
+ return -EINVAL;
+
+ return count;
+}
+
+static DEVICE_ATTR_WO(mcdi_logging);
+
static int xlnx_cdx_probe(struct platform_device *pdev)
{
struct cdx_controller *cdx;
@@ -154,6 +171,9 @@ static int xlnx_cdx_probe(struct platform_device *pdev)
cdx->priv = cdx_mcdi;
cdx->ops = &cdx_ops;
+ if (device_create_file(&pdev->dev, &dev_attr_mcdi_logging))
+ dev_warn(&pdev->dev, "Failed to create sysfs file\n");
+
ret = cdx_setup_rpmsg(pdev);
if (ret) {
if (ret != -EPROBE_DEFER)
@@ -181,6 +201,7 @@ static int xlnx_cdx_remove(struct platform_device *pdev)
cdx_destroy_rpmsg(pdev);
+ device_remove_file(&pdev->dev, &dev_attr_mcdi_logging);
kfree(cdx);
cdx_mcdi_finish(cdx_mcdi);
@@ -31,9 +31,7 @@ struct cdx_mcdi_copy_buffer {
struct cdx_dword buffer[DIV_ROUND_UP(MCDI_CTL_SDU_LEN_MAX, 4)];
};
-#ifdef CONFIG_MCDI_LOGGING
#define LOG_LINE_MAX (1024 - 32)
-#endif
static void cdx_mcdi_cancel_cmd(struct cdx_mcdi *cdx, struct cdx_mcdi_cmd *cmd);
static void cdx_mcdi_wait_for_cleanup(struct cdx_mcdi *cdx);
@@ -118,12 +116,12 @@ int cdx_mcdi_init(struct cdx_mcdi *cdx)
mcdi = cdx_mcdi_if(cdx);
mcdi->cdx = cdx;
+ mcdi->logging_enabled = !!CDX_MCDI_LOGGING;
-#ifdef CONFIG_MCDI_LOGGING
mcdi->logging_buffer = kmalloc(LOG_LINE_MAX, GFP_KERNEL);
if (!mcdi->logging_buffer)
goto fail2;
-#endif
+
mcdi->workqueue = alloc_ordered_workqueue("mcdi_wq", 0);
if (!mcdi->workqueue)
goto fail3;
@@ -136,10 +134,8 @@ int cdx_mcdi_init(struct cdx_mcdi *cdx)
return 0;
fail3:
-#ifdef CONFIG_MCDI_LOGGING
kfree(mcdi->logging_buffer);
fail2:
-#endif
kfree(cdx->mcdi);
cdx->mcdi = NULL;
fail:
@@ -155,16 +151,25 @@ void cdx_mcdi_finish(struct cdx_mcdi *cdx)
return;
cdx_mcdi_wait_for_cleanup(cdx);
-
-#ifdef CONFIG_MCDI_LOGGING
kfree(mcdi->logging_buffer);
-#endif
destroy_workqueue(mcdi->workqueue);
kfree(cdx->mcdi);
cdx->mcdi = NULL;
}
+int cdx_enable_mcdi_logging(struct cdx_mcdi *cdx, bool enable)
+{
+ struct cdx_mcdi_iface *mcdi;
+
+ mcdi = cdx_mcdi_if(cdx);
+ if (!mcdi)
+ return -EINVAL;
+
+ mcdi->logging_enabled = enable;
+ return 0;
+}
+
static bool cdx_mcdi_flushed(struct cdx_mcdi_iface *mcdi, bool ignore_cleanups)
{
bool flushed;
@@ -246,15 +251,9 @@ static void cdx_mcdi_send_request(struct cdx_mcdi *cdx,
size_t hdr_len;
bool not_epoch;
u32 xflags;
-#ifdef CONFIG_MCDI_LOGGING
- char *buf;
-#endif
if (!mcdi)
return;
-#ifdef CONFIG_MCDI_LOGGING
- buf = mcdi->logging_buffer; /* page-sized */
-#endif
mcdi->prev_seq = cmd->seq;
mcdi->seq_held_by[cmd->seq] = cmd;
@@ -281,10 +280,10 @@ static void cdx_mcdi_send_request(struct cdx_mcdi *cdx,
MC_CMD_V2_EXTN_IN_MCDI_MESSAGE_TYPE_PLATFORM);
hdr_len = 8;
-#ifdef CONFIG_MCDI_LOGGING
- if (!WARN_ON_ONCE(!buf)) {
+ if (mcdi->logging_enabled) {
const struct cdx_dword *frags[] = { hdr, inbuf };
const size_t frag_len[] = { hdr_len, round_up(inlen, 4) };
+ char *log = mcdi->logging_buffer;
int bytes = 0;
int i, j;
@@ -300,18 +299,18 @@ static void cdx_mcdi_send_request(struct cdx_mcdi *cdx,
* The string before that is just over 70 bytes.
*/
if ((bytes + 75) > LOG_LINE_MAX) {
- pr_info("MCDI RPC REQ:%s \\\n", buf);
+ pr_info("MCDI RPC REQ:%s \\\n", log);
bytes = 0;
}
- bytes += snprintf(buf + bytes,
+ bytes += snprintf(log + bytes,
LOG_LINE_MAX - bytes, " %08x",
le32_to_cpu(frag[i].cdx_u32));
}
}
- pr_info("MCDI RPC REQ:%s\n", buf);
+ pr_info("MCDI RPC REQ:%s\n", log);
}
-#endif
+
hdr[0].cdx_u32 |= (__force __le32)(cdx_mcdi_payload_csum(hdr, hdr_len, inbuf, inlen) <<
MCDI_HEADER_XFLAGS_LBN);
cdx->mcdi_ops->mcdi_request(cdx, hdr, hdr_len, inbuf, inlen);
@@ -700,8 +699,7 @@ static bool cdx_mcdi_complete_cmd(struct cdx_mcdi_iface *mcdi,
resp_data_len = 0;
}
-#ifdef CONFIG_MCDI_LOGGING
- if (!WARN_ON_ONCE(!mcdi->logging_buffer)) {
+ if (mcdi->logging_enabled) {
char *log = mcdi->logging_buffer;
int i, bytes = 0;
size_t rlen;
@@ -721,7 +719,6 @@ static bool cdx_mcdi_complete_cmd(struct cdx_mcdi_iface *mcdi,
pr_info("MCDI RPC RESP:%s\n", log);
}
-#endif
if (error && resp_data_len == 0) {
/* MC rebooted during command */
@@ -22,6 +22,11 @@
#define CDX_WARN_ON_PARANOID(x) do {} while (0)
#endif
+#ifdef CONFIG_CDX_MCDI_LOGGING
+#define CDX_MCDI_LOGGING 1
+#else
+#define CDX_MCDI_LOGGING 0
+#endif
/**
* enum cdx_mcdi_mode - MCDI transaction mode
* @MCDI_MODE_EVENTS: wait for an mcdi response callback.
@@ -170,10 +175,8 @@ struct cdx_mcdi_iface {
enum cdx_mcdi_mode mode;
u8 prev_seq;
bool new_epoch;
-#ifdef CONFIG_MCDI_LOGGING
bool logging_enabled;
char *logging_buffer;
-#endif
};
/**
@@ -193,6 +196,7 @@ static inline struct cdx_mcdi_iface *cdx_mcdi_if(struct cdx_mcdi *cdx)
int cdx_mcdi_init(struct cdx_mcdi *cdx);
void cdx_mcdi_finish(struct cdx_mcdi *cdx);
+int cdx_enable_mcdi_logging(struct cdx_mcdi *cdx, bool enable);
void cdx_mcdi_process_cmd(struct cdx_mcdi *cdx, struct cdx_dword *outbuf, int len);
int cdx_mcdi_rpc(struct cdx_mcdi *cdx, unsigned int cmd,