| Message ID | 20241023063245.1404420-5-Shyam-sundar.S-k@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | platform/x86/amd/pmf: Updates to AMD PMF driver | expand |
On 10/23/2024 01:32, Shyam Sundar S K wrote: > Use platform_get_resource() to fetch the memory resource instead of > acpi_walk_resources() and devm_ioremap_resource() for mapping the > resources. > > PS: We cannot use resource_size() here because it adds an extra byte to round > off the size. In the case of PMF ResourceTemplate(), this rounding is > already handled within the _CRS. Using resource_size() would increase the > resource size by 1, causing a mismatch with the length field and leading > to issues. Therefore, simply use end-start of the ACPI resource to obtain > the actual length. > > Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> > Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > --- > drivers/platform/x86/amd/pmf/Kconfig | 1 + > drivers/platform/x86/amd/pmf/acpi.c | 46 +++++++++++---------------- > drivers/platform/x86/amd/pmf/pmf.h | 6 ++-- > drivers/platform/x86/amd/pmf/tee-if.c | 8 ++--- > 4 files changed, 28 insertions(+), 33 deletions(-) > > diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig > index f4fa8bd8bda8..99d67cdbd91e 100644 > --- a/drivers/platform/x86/amd/pmf/Kconfig > +++ b/drivers/platform/x86/amd/pmf/Kconfig > @@ -11,6 +11,7 @@ config AMD_PMF > select ACPI_PLATFORM_PROFILE > depends on TEE && AMDTEE > depends on AMD_SFH_HID > + depends on HAS_IOMEM > help > This driver provides support for the AMD Platform Management Framework. > The goal is to enhance end user experience by making AMD PCs smarter, > diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c > index d5b496433d69..62f984fe40c6 100644 > --- a/drivers/platform/x86/amd/pmf/acpi.c > +++ b/drivers/platform/x86/amd/pmf/acpi.c > @@ -433,37 +433,29 @@ int apmf_install_handler(struct amd_pmf_dev *pmf_dev) > return 0; > } > > -static acpi_status apmf_walk_resources(struct acpi_resource *res, void *data) > +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) > { > - struct amd_pmf_dev *dev = data; > + struct platform_device *pdev = to_platform_device(pmf_dev->dev); > > - switch (res->type) { > - case ACPI_RESOURCE_TYPE_ADDRESS64: > - dev->policy_addr = res->data.address64.address.minimum; > - dev->policy_sz = res->data.address64.address.address_length; > - break; > - case ACPI_RESOURCE_TYPE_FIXED_MEMORY32: > - dev->policy_addr = res->data.fixed_memory32.address; > - dev->policy_sz = res->data.fixed_memory32.address_length; > - break; > - } > - > - if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ || dev->policy_sz == 0) { > - pr_err("Incorrect Policy params, possibly a SBIOS bug\n"); > - return AE_ERROR; > + pmf_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!pmf_dev->res) { > + dev_err(pmf_dev->dev, "Failed to get I/O memory resource\n"); > + return -EINVAL; > } > > - return AE_OK; > -} > - > -int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) > -{ > - acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev); > - acpi_status status; > - > - status = acpi_walk_resources(ahandle, METHOD_NAME__CRS, apmf_walk_resources, pmf_dev); > - if (ACPI_FAILURE(status)) { > - dev_dbg(pmf_dev->dev, "acpi_walk_resources failed :%d\n", status); > + pmf_dev->policy_addr = pmf_dev->res->start; > + /* > + * We cannot use resource_size() here because it adds an extra byte to round off the size. > + * In the case of PMF ResourceTemplate(), this rounding is already handled within the _CRS. > + * Using resource_size() would increase the resource size by 1, causing a mismatch with the > + * length field and leading to issues. Therefore, simply use end-start of the ACPI resource > + * to obtain the actual length. > + */ > + pmf_dev->policy_sz = pmf_dev->res->end - pmf_dev->res->start; > + > + if (!pmf_dev->policy_addr || pmf_dev->policy_sz > POLICY_BUF_MAX_SZ || > + pmf_dev->policy_sz == 0) { > + dev_err(pmf_dev->dev, "Incorrect policy params, possibly a SBIOS bug\n"); This upgrades the previous message from debug to error. TL;DR I feel this error should stay as dev_dbg() if no function checks are present for Smart PC. I don't think it's necessarily an error though. Smart PC checks are a bit different than the other checks. There isn't a check for a bit being set to indicate the function is supported. So if the BIOS has the declaration for the region but it's not populated it might not have a Smart PC policy and this shouldn't be reported as a BIOS bug. > return -EINVAL; > } > > diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h > index 8ce8816da9c1..a79808fda1d8 100644 > --- a/drivers/platform/x86/amd/pmf/pmf.h > +++ b/drivers/platform/x86/amd/pmf/pmf.h > @@ -13,6 +13,7 @@ > > #include <linux/acpi.h> > #include <linux/input.h> > +#include <linux/platform_device.h> > #include <linux/platform_profile.h> > > #define POLICY_BUF_MAX_SZ 0x4b000 > @@ -355,19 +356,20 @@ struct amd_pmf_dev { > /* Smart PC solution builder */ > struct dentry *esbin; > unsigned char *policy_buf; > - u32 policy_sz; > + resource_size_t policy_sz; > struct tee_context *tee_ctx; > struct tee_shm *fw_shm_pool; > u32 session_id; > void *shbuf; > struct delayed_work pb_work; > struct pmf_action_table *prev_data; > - u64 policy_addr; > + resource_size_t policy_addr; > void __iomem *policy_base; > bool smart_pc_enabled; > u16 pmf_if_version; > struct input_dev *pmf_idev; > size_t mtable_size; > + struct resource *res; > }; > > struct apmf_sps_prop_granular_v2 { > diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c > index 19c27b6e4666..555b8d6314e0 100644 > --- a/drivers/platform/x86/amd/pmf/tee-if.c > +++ b/drivers/platform/x86/amd/pmf/tee-if.c > @@ -257,7 +257,7 @@ static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev) > return -ENODEV; > } > > - dev_dbg(dev->dev, "Policy Binary size: %u bytes\n", dev->policy_sz); > + dev_dbg(dev->dev, "Policy Binary size: %llu bytes\n", dev->policy_sz); > memset(dev->shbuf, 0, dev->policy_sz); > ta_sm = dev->shbuf; > in = &ta_sm->pmf_input.init_table; > @@ -512,9 +512,9 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) > if (ret) > goto error; > > - dev->policy_base = devm_ioremap(dev->dev, dev->policy_addr, dev->policy_sz); > - if (!dev->policy_base) { > - ret = -ENOMEM; > + dev->policy_base = devm_ioremap_resource(dev->dev, dev->res); > + if (IS_ERR(dev->policy_base)) { > + ret = PTR_ERR(dev->policy_base); > goto error; > } >
On 10/23/2024 19:35, Mario Limonciello wrote: > On 10/23/2024 01:32, Shyam Sundar S K wrote: >> Use platform_get_resource() to fetch the memory resource instead of >> acpi_walk_resources() and devm_ioremap_resource() for mapping the >> resources. >> >> PS: We cannot use resource_size() here because it adds an extra byte >> to round >> off the size. In the case of PMF ResourceTemplate(), this rounding is >> already handled within the _CRS. Using resource_size() would >> increase the >> resource size by 1, causing a mismatch with the length field and >> leading >> to issues. Therefore, simply use end-start of the ACPI resource to >> obtain >> the actual length. >> >> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> >> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> >> --- >> drivers/platform/x86/amd/pmf/Kconfig | 1 + >> drivers/platform/x86/amd/pmf/acpi.c | 46 >> +++++++++++---------------- >> drivers/platform/x86/amd/pmf/pmf.h | 6 ++-- >> drivers/platform/x86/amd/pmf/tee-if.c | 8 ++--- >> 4 files changed, 28 insertions(+), 33 deletions(-) >> >> diff --git a/drivers/platform/x86/amd/pmf/Kconfig >> b/drivers/platform/x86/amd/pmf/Kconfig >> index f4fa8bd8bda8..99d67cdbd91e 100644 >> --- a/drivers/platform/x86/amd/pmf/Kconfig >> +++ b/drivers/platform/x86/amd/pmf/Kconfig >> @@ -11,6 +11,7 @@ config AMD_PMF >> select ACPI_PLATFORM_PROFILE >> depends on TEE && AMDTEE >> depends on AMD_SFH_HID >> + depends on HAS_IOMEM >> help >> This driver provides support for the AMD Platform Management >> Framework. >> The goal is to enhance end user experience by making AMD PCs >> smarter, >> diff --git a/drivers/platform/x86/amd/pmf/acpi.c >> b/drivers/platform/x86/amd/pmf/acpi.c >> index d5b496433d69..62f984fe40c6 100644 >> --- a/drivers/platform/x86/amd/pmf/acpi.c >> +++ b/drivers/platform/x86/amd/pmf/acpi.c >> @@ -433,37 +433,29 @@ int apmf_install_handler(struct amd_pmf_dev >> *pmf_dev) >> return 0; >> } >> -static acpi_status apmf_walk_resources(struct acpi_resource *res, >> void *data) >> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) >> { >> - struct amd_pmf_dev *dev = data; >> + struct platform_device *pdev = to_platform_device(pmf_dev->dev); >> - switch (res->type) { >> - case ACPI_RESOURCE_TYPE_ADDRESS64: >> - dev->policy_addr = res->data.address64.address.minimum; >> - dev->policy_sz = res->data.address64.address.address_length; >> - break; >> - case ACPI_RESOURCE_TYPE_FIXED_MEMORY32: >> - dev->policy_addr = res->data.fixed_memory32.address; >> - dev->policy_sz = res->data.fixed_memory32.address_length; >> - break; >> - } >> - >> - if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ || >> dev->policy_sz == 0) { >> - pr_err("Incorrect Policy params, possibly a SBIOS bug\n"); >> - return AE_ERROR; >> + pmf_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + if (!pmf_dev->res) { >> + dev_err(pmf_dev->dev, "Failed to get I/O memory resource\n"); here ^^^^^^^ >> + return -EINVAL; >> } >> - return AE_OK; >> -} >> - >> -int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) >> -{ >> - acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev); >> - acpi_status status; >> - >> - status = acpi_walk_resources(ahandle, METHOD_NAME__CRS, >> apmf_walk_resources, pmf_dev); >> - if (ACPI_FAILURE(status)) { >> - dev_dbg(pmf_dev->dev, "acpi_walk_resources failed :%d\n", >> status); >> + pmf_dev->policy_addr = pmf_dev->res->start; >> + /* >> + * We cannot use resource_size() here because it adds an extra >> byte to round off the size. >> + * In the case of PMF ResourceTemplate(), this rounding is >> already handled within the _CRS. >> + * Using resource_size() would increase the resource size by 1, >> causing a mismatch with the >> + * length field and leading to issues. Therefore, simply use >> end-start of the ACPI resource >> + * to obtain the actual length. >> + */ >> + pmf_dev->policy_sz = pmf_dev->res->end - pmf_dev->res->start; >> + >> + if (!pmf_dev->policy_addr || pmf_dev->policy_sz > >> POLICY_BUF_MAX_SZ || >> + pmf_dev->policy_sz == 0) { >> + dev_err(pmf_dev->dev, "Incorrect policy params, possibly a >> SBIOS bug\n"); > > This upgrades the previous message from debug to error. It is dev_err() even before this change. > > TL;DR I feel this error should stay as dev_dbg() if no function checks > are present for Smart PC. > > I don't think it's necessarily an error though. > Smart PC checks are a bit different than the other checks. There > isn't a check for a bit being set to indicate the function is supported. > > So if the BIOS has the declaration for the region but it's not > populated it might not have a Smart PC policy and this shouldn't be > reported as a BIOS bug. This should be included in the CPM package, and the BIOS team is responsible for packaging a policy binary. From a driver design standpoint, the absence of the policy binary should be treated as an error, as there's no reason for the BIOS to advertise the Smart PC bits without providing the policy binary. Therefore, this should trigger a `dev_err()` and be considered a BIOS bug. Thanks, Shyam > >> return -EINVAL; >> } >> diff --git a/drivers/platform/x86/amd/pmf/pmf.h >> b/drivers/platform/x86/amd/pmf/pmf.h >> index 8ce8816da9c1..a79808fda1d8 100644 >> --- a/drivers/platform/x86/amd/pmf/pmf.h >> +++ b/drivers/platform/x86/amd/pmf/pmf.h >> @@ -13,6 +13,7 @@ >> #include <linux/acpi.h> >> #include <linux/input.h> >> +#include <linux/platform_device.h> >> #include <linux/platform_profile.h> >> #define POLICY_BUF_MAX_SZ 0x4b000 >> @@ -355,19 +356,20 @@ struct amd_pmf_dev { >> /* Smart PC solution builder */ >> struct dentry *esbin; >> unsigned char *policy_buf; >> - u32 policy_sz; >> + resource_size_t policy_sz; >> struct tee_context *tee_ctx; >> struct tee_shm *fw_shm_pool; >> u32 session_id; >> void *shbuf; >> struct delayed_work pb_work; >> struct pmf_action_table *prev_data; >> - u64 policy_addr; >> + resource_size_t policy_addr; >> void __iomem *policy_base; >> bool smart_pc_enabled; >> u16 pmf_if_version; >> struct input_dev *pmf_idev; >> size_t mtable_size; >> + struct resource *res; >> }; >> struct apmf_sps_prop_granular_v2 { >> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c >> b/drivers/platform/x86/amd/pmf/tee-if.c >> index 19c27b6e4666..555b8d6314e0 100644 >> --- a/drivers/platform/x86/amd/pmf/tee-if.c >> +++ b/drivers/platform/x86/amd/pmf/tee-if.c >> @@ -257,7 +257,7 @@ static int amd_pmf_invoke_cmd_init(struct >> amd_pmf_dev *dev) >> return -ENODEV; >> } >> - dev_dbg(dev->dev, "Policy Binary size: %u bytes\n", >> dev->policy_sz); >> + dev_dbg(dev->dev, "Policy Binary size: %llu bytes\n", >> dev->policy_sz); >> memset(dev->shbuf, 0, dev->policy_sz); >> ta_sm = dev->shbuf; >> in = &ta_sm->pmf_input.init_table; >> @@ -512,9 +512,9 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) >> if (ret) >> goto error; >> - dev->policy_base = devm_ioremap(dev->dev, dev->policy_addr, >> dev->policy_sz); >> - if (!dev->policy_base) { >> - ret = -ENOMEM; >> + dev->policy_base = devm_ioremap_resource(dev->dev, dev->res); >> + if (IS_ERR(dev->policy_base)) { >> + ret = PTR_ERR(dev->policy_base); >> goto error; >> } >> >
On 10/23/2024 09:37, Shyam Sundar S K wrote: > > > On 10/23/2024 19:35, Mario Limonciello wrote: >> On 10/23/2024 01:32, Shyam Sundar S K wrote: >>> Use platform_get_resource() to fetch the memory resource instead of >>> acpi_walk_resources() and devm_ioremap_resource() for mapping the >>> resources. >>> >>> PS: We cannot use resource_size() here because it adds an extra byte >>> to round >>> off the size. In the case of PMF ResourceTemplate(), this rounding is >>> already handled within the _CRS. Using resource_size() would >>> increase the >>> resource size by 1, causing a mismatch with the length field and >>> leading >>> to issues. Therefore, simply use end-start of the ACPI resource to >>> obtain >>> the actual length. >>> >>> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> >>> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> >>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> >>> --- >>> drivers/platform/x86/amd/pmf/Kconfig | 1 + >>> drivers/platform/x86/amd/pmf/acpi.c | 46 >>> +++++++++++---------------- >>> drivers/platform/x86/amd/pmf/pmf.h | 6 ++-- >>> drivers/platform/x86/amd/pmf/tee-if.c | 8 ++--- >>> 4 files changed, 28 insertions(+), 33 deletions(-) >>> >>> diff --git a/drivers/platform/x86/amd/pmf/Kconfig >>> b/drivers/platform/x86/amd/pmf/Kconfig >>> index f4fa8bd8bda8..99d67cdbd91e 100644 >>> --- a/drivers/platform/x86/amd/pmf/Kconfig >>> +++ b/drivers/platform/x86/amd/pmf/Kconfig >>> @@ -11,6 +11,7 @@ config AMD_PMF >>> select ACPI_PLATFORM_PROFILE >>> depends on TEE && AMDTEE >>> depends on AMD_SFH_HID >>> + depends on HAS_IOMEM >>> help >>> This driver provides support for the AMD Platform Management >>> Framework. >>> The goal is to enhance end user experience by making AMD PCs >>> smarter, >>> diff --git a/drivers/platform/x86/amd/pmf/acpi.c >>> b/drivers/platform/x86/amd/pmf/acpi.c >>> index d5b496433d69..62f984fe40c6 100644 >>> --- a/drivers/platform/x86/amd/pmf/acpi.c >>> +++ b/drivers/platform/x86/amd/pmf/acpi.c >>> @@ -433,37 +433,29 @@ int apmf_install_handler(struct amd_pmf_dev >>> *pmf_dev) >>> return 0; >>> } >>> -static acpi_status apmf_walk_resources(struct acpi_resource *res, >>> void *data) >>> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) >>> { >>> - struct amd_pmf_dev *dev = data; >>> + struct platform_device *pdev = to_platform_device(pmf_dev->dev); >>> - switch (res->type) { >>> - case ACPI_RESOURCE_TYPE_ADDRESS64: >>> - dev->policy_addr = res->data.address64.address.minimum; >>> - dev->policy_sz = res->data.address64.address.address_length; >>> - break; >>> - case ACPI_RESOURCE_TYPE_FIXED_MEMORY32: >>> - dev->policy_addr = res->data.fixed_memory32.address; >>> - dev->policy_sz = res->data.fixed_memory32.address_length; >>> - break; >>> - } >>> - >>> - if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ || >>> dev->policy_sz == 0) { >>> - pr_err("Incorrect Policy params, possibly a SBIOS bug\n"); >>> - return AE_ERROR; >>> + pmf_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> + if (!pmf_dev->res) { >>> + dev_err(pmf_dev->dev, "Failed to get I/O memory resource\n"); > > here ^^^^^^^ > >>> + return -EINVAL; >>> } >>> - return AE_OK; >>> -} >>> - >>> -int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) >>> -{ >>> - acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev); >>> - acpi_status status; >>> - >>> - status = acpi_walk_resources(ahandle, METHOD_NAME__CRS, >>> apmf_walk_resources, pmf_dev); >>> - if (ACPI_FAILURE(status)) { >>> - dev_dbg(pmf_dev->dev, "acpi_walk_resources failed :%d\n", >>> status); >>> + pmf_dev->policy_addr = pmf_dev->res->start; >>> + /* >>> + * We cannot use resource_size() here because it adds an extra >>> byte to round off the size. >>> + * In the case of PMF ResourceTemplate(), this rounding is >>> already handled within the _CRS. >>> + * Using resource_size() would increase the resource size by 1, >>> causing a mismatch with the >>> + * length field and leading to issues. Therefore, simply use >>> end-start of the ACPI resource >>> + * to obtain the actual length. >>> + */ >>> + pmf_dev->policy_sz = pmf_dev->res->end - pmf_dev->res->start; >>> + >>> + if (!pmf_dev->policy_addr || pmf_dev->policy_sz > >>> POLICY_BUF_MAX_SZ || >>> + pmf_dev->policy_sz == 0) { >>> + dev_err(pmf_dev->dev, "Incorrect policy params, possibly a >>> SBIOS bug\n"); >> >> This upgrades the previous message from debug to error. > > It is dev_err() even before this change. > >> >> TL;DR I feel this error should stay as dev_dbg() if no function checks >> are present for Smart PC. >> >> I don't think it's necessarily an error though. >> Smart PC checks are a bit different than the other checks. There >> isn't a check for a bit being set to indicate the function is supported. >> >> So if the BIOS has the declaration for the region but it's not >> populated it might not have a Smart PC policy and this shouldn't be >> reported as a BIOS bug. > > This should be included in the CPM package, and the BIOS team is > responsible for packaging a policy binary. > > From a driver design standpoint, the absence of the policy binary > should be treated as an error, as there's no reason for the BIOS to > advertise the Smart PC bits without providing the policy binary. > > Therefore, this should trigger a `dev_err()` and be considered a BIOS bug. > OK I agree with this specific error, but I took a closer look at the bug associated with 03cea821b82cb ("platform/x86/amd: pmf: Decrease error message to debug") As _CRS is patched out by BIOS I suspect that system will now start showing: dev_err(pmf_dev->dev, "Failed to get I/O memory resource\n"); So how exactly is a platform designer supposed to not advertise smart PC bits? It seems the only check is the presence of that resource. > Thanks, > Shyam > >> >>> return -EINVAL; >>> } >>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h >>> b/drivers/platform/x86/amd/pmf/pmf.h >>> index 8ce8816da9c1..a79808fda1d8 100644 >>> --- a/drivers/platform/x86/amd/pmf/pmf.h >>> +++ b/drivers/platform/x86/amd/pmf/pmf.h >>> @@ -13,6 +13,7 @@ >>> #include <linux/acpi.h> >>> #include <linux/input.h> >>> +#include <linux/platform_device.h> >>> #include <linux/platform_profile.h> >>> #define POLICY_BUF_MAX_SZ 0x4b000 >>> @@ -355,19 +356,20 @@ struct amd_pmf_dev { >>> /* Smart PC solution builder */ >>> struct dentry *esbin; >>> unsigned char *policy_buf; >>> - u32 policy_sz; >>> + resource_size_t policy_sz; >>> struct tee_context *tee_ctx; >>> struct tee_shm *fw_shm_pool; >>> u32 session_id; >>> void *shbuf; >>> struct delayed_work pb_work; >>> struct pmf_action_table *prev_data; >>> - u64 policy_addr; >>> + resource_size_t policy_addr; >>> void __iomem *policy_base; >>> bool smart_pc_enabled; >>> u16 pmf_if_version; >>> struct input_dev *pmf_idev; >>> size_t mtable_size; >>> + struct resource *res; >>> }; >>> struct apmf_sps_prop_granular_v2 { >>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c >>> b/drivers/platform/x86/amd/pmf/tee-if.c >>> index 19c27b6e4666..555b8d6314e0 100644 >>> --- a/drivers/platform/x86/amd/pmf/tee-if.c >>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c >>> @@ -257,7 +257,7 @@ static int amd_pmf_invoke_cmd_init(struct >>> amd_pmf_dev *dev) >>> return -ENODEV; >>> } >>> - dev_dbg(dev->dev, "Policy Binary size: %u bytes\n", >>> dev->policy_sz); >>> + dev_dbg(dev->dev, "Policy Binary size: %llu bytes\n", >>> dev->policy_sz); >>> memset(dev->shbuf, 0, dev->policy_sz); >>> ta_sm = dev->shbuf; >>> in = &ta_sm->pmf_input.init_table; >>> @@ -512,9 +512,9 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) >>> if (ret) >>> goto error; >>> - dev->policy_base = devm_ioremap(dev->dev, dev->policy_addr, >>> dev->policy_sz); >>> - if (!dev->policy_base) { >>> - ret = -ENOMEM; >>> + dev->policy_base = devm_ioremap_resource(dev->dev, dev->res); >>> + if (IS_ERR(dev->policy_base)) { >>> + ret = PTR_ERR(dev->policy_base); >>> goto error; >>> } >>> >>
On 10/23/2024 20:20, Mario Limonciello wrote: > On 10/23/2024 09:37, Shyam Sundar S K wrote: >> >> >> On 10/23/2024 19:35, Mario Limonciello wrote: >>> On 10/23/2024 01:32, Shyam Sundar S K wrote: >>>> Use platform_get_resource() to fetch the memory resource instead of >>>> acpi_walk_resources() and devm_ioremap_resource() for mapping the >>>> resources. >>>> >>>> PS: We cannot use resource_size() here because it adds an extra byte >>>> to round >>>> off the size. In the case of PMF ResourceTemplate(), this rounding is >>>> already handled within the _CRS. Using resource_size() would >>>> increase the >>>> resource size by 1, causing a mismatch with the length field and >>>> leading >>>> to issues. Therefore, simply use end-start of the ACPI resource to >>>> obtain >>>> the actual length. >>>> >>>> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> >>>> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> >>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> >>>> --- >>>> drivers/platform/x86/amd/pmf/Kconfig | 1 + >>>> drivers/platform/x86/amd/pmf/acpi.c | 46 >>>> +++++++++++---------------- >>>> drivers/platform/x86/amd/pmf/pmf.h | 6 ++-- >>>> drivers/platform/x86/amd/pmf/tee-if.c | 8 ++--- >>>> 4 files changed, 28 insertions(+), 33 deletions(-) >>>> >>>> diff --git a/drivers/platform/x86/amd/pmf/Kconfig >>>> b/drivers/platform/x86/amd/pmf/Kconfig >>>> index f4fa8bd8bda8..99d67cdbd91e 100644 >>>> --- a/drivers/platform/x86/amd/pmf/Kconfig >>>> +++ b/drivers/platform/x86/amd/pmf/Kconfig >>>> @@ -11,6 +11,7 @@ config AMD_PMF >>>> select ACPI_PLATFORM_PROFILE >>>> depends on TEE && AMDTEE >>>> depends on AMD_SFH_HID >>>> + depends on HAS_IOMEM >>>> help >>>> This driver provides support for the AMD Platform Management >>>> Framework. >>>> The goal is to enhance end user experience by making AMD PCs >>>> smarter, >>>> diff --git a/drivers/platform/x86/amd/pmf/acpi.c >>>> b/drivers/platform/x86/amd/pmf/acpi.c >>>> index d5b496433d69..62f984fe40c6 100644 >>>> --- a/drivers/platform/x86/amd/pmf/acpi.c >>>> +++ b/drivers/platform/x86/amd/pmf/acpi.c >>>> @@ -433,37 +433,29 @@ int apmf_install_handler(struct amd_pmf_dev >>>> *pmf_dev) >>>> return 0; >>>> } >>>> -static acpi_status apmf_walk_resources(struct acpi_resource *res, >>>> void *data) >>>> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) >>>> { >>>> - struct amd_pmf_dev *dev = data; >>>> + struct platform_device *pdev = to_platform_device(pmf_dev->dev); >>>> - switch (res->type) { >>>> - case ACPI_RESOURCE_TYPE_ADDRESS64: >>>> - dev->policy_addr = res->data.address64.address.minimum; >>>> - dev->policy_sz = res->data.address64.address.address_length; >>>> - break; >>>> - case ACPI_RESOURCE_TYPE_FIXED_MEMORY32: >>>> - dev->policy_addr = res->data.fixed_memory32.address; >>>> - dev->policy_sz = res->data.fixed_memory32.address_length; >>>> - break; >>>> - } >>>> - >>>> - if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ || >>>> dev->policy_sz == 0) { >>>> - pr_err("Incorrect Policy params, possibly a SBIOS bug\n"); >>>> - return AE_ERROR; >>>> + pmf_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>> + if (!pmf_dev->res) { >>>> + dev_err(pmf_dev->dev, "Failed to get I/O memory >>>> resource\n"); >> >> here ^^^^^^^ >> >>>> + return -EINVAL; >>>> } >>>> - return AE_OK; >>>> -} >>>> - >>>> -int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) >>>> -{ >>>> - acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev); >>>> - acpi_status status; >>>> - >>>> - status = acpi_walk_resources(ahandle, METHOD_NAME__CRS, >>>> apmf_walk_resources, pmf_dev); >>>> - if (ACPI_FAILURE(status)) { >>>> - dev_dbg(pmf_dev->dev, "acpi_walk_resources failed :%d\n", >>>> status); >>>> + pmf_dev->policy_addr = pmf_dev->res->start; >>>> + /* >>>> + * We cannot use resource_size() here because it adds an extra >>>> byte to round off the size. >>>> + * In the case of PMF ResourceTemplate(), this rounding is >>>> already handled within the _CRS. >>>> + * Using resource_size() would increase the resource size by 1, >>>> causing a mismatch with the >>>> + * length field and leading to issues. Therefore, simply use >>>> end-start of the ACPI resource >>>> + * to obtain the actual length. >>>> + */ >>>> + pmf_dev->policy_sz = pmf_dev->res->end - pmf_dev->res->start; >>>> + >>>> + if (!pmf_dev->policy_addr || pmf_dev->policy_sz > >>>> POLICY_BUF_MAX_SZ || >>>> + pmf_dev->policy_sz == 0) { >>>> + dev_err(pmf_dev->dev, "Incorrect policy params, possibly a >>>> SBIOS bug\n"); >>> >>> This upgrades the previous message from debug to error. >> >> It is dev_err() even before this change. >> >>> >>> TL;DR I feel this error should stay as dev_dbg() if no function checks >>> are present for Smart PC. >>> >>> I don't think it's necessarily an error though. >>> Smart PC checks are a bit different than the other checks. There >>> isn't a check for a bit being set to indicate the function is >>> supported. >>> >>> So if the BIOS has the declaration for the region but it's not >>> populated it might not have a Smart PC policy and this shouldn't be >>> reported as a BIOS bug. >> >> This should be included in the CPM package, and the BIOS team is >> responsible for packaging a policy binary. >> >> From a driver design standpoint, the absence of the policy binary >> should be treated as an error, as there's no reason for the BIOS to >> advertise the Smart PC bits without providing the policy binary. >> >> Therefore, this should trigger a `dev_err()` and be considered a >> BIOS bug. >> > > OK I agree with this specific error, but I took a closer look at the > bug associated with > 03cea821b82cb ("platform/x86/amd: pmf: Decrease error message to debug") ah! but your comment was just inline to: dev_err(pmf_dev->dev, "Incorrect policy params, possibly a SBIOS bug\n"); So, I was thinking you are saying to downgrade this to dev_dbg() and hence the above clarification. if the comment is for: dev_err(pmf_dev->dev, "Failed to get I/O memory resource\n"); then Yes, I agree we should have dev_dbg() and I will respin a new version. Thanks, Shyam > > As _CRS is patched out by BIOS I suspect that system will now start > showing: > > dev_err(pmf_dev->dev, "Failed to get I/O memory resource\n"); > > So how exactly is a platform designer supposed to not advertise smart > PC bits? It seems the only check is the presence of that resource. > >> Thanks, >> Shyam >> >>> >>>> return -EINVAL; >>>> } >>>> diff --git a/drivers/platform/x86/amd/pmf/pmf.h >>>> b/drivers/platform/x86/amd/pmf/pmf.h >>>> index 8ce8816da9c1..a79808fda1d8 100644 >>>> --- a/drivers/platform/x86/amd/pmf/pmf.h >>>> +++ b/drivers/platform/x86/amd/pmf/pmf.h >>>> @@ -13,6 +13,7 @@ >>>> #include <linux/acpi.h> >>>> #include <linux/input.h> >>>> +#include <linux/platform_device.h> >>>> #include <linux/platform_profile.h> >>>> #define POLICY_BUF_MAX_SZ 0x4b000 >>>> @@ -355,19 +356,20 @@ struct amd_pmf_dev { >>>> /* Smart PC solution builder */ >>>> struct dentry *esbin; >>>> unsigned char *policy_buf; >>>> - u32 policy_sz; >>>> + resource_size_t policy_sz; >>>> struct tee_context *tee_ctx; >>>> struct tee_shm *fw_shm_pool; >>>> u32 session_id; >>>> void *shbuf; >>>> struct delayed_work pb_work; >>>> struct pmf_action_table *prev_data; >>>> - u64 policy_addr; >>>> + resource_size_t policy_addr; >>>> void __iomem *policy_base; >>>> bool smart_pc_enabled; >>>> u16 pmf_if_version; >>>> struct input_dev *pmf_idev; >>>> size_t mtable_size; >>>> + struct resource *res; >>>> }; >>>> struct apmf_sps_prop_granular_v2 { >>>> diff --git a/drivers/platform/x86/amd/pmf/tee-if.c >>>> b/drivers/platform/x86/amd/pmf/tee-if.c >>>> index 19c27b6e4666..555b8d6314e0 100644 >>>> --- a/drivers/platform/x86/amd/pmf/tee-if.c >>>> +++ b/drivers/platform/x86/amd/pmf/tee-if.c >>>> @@ -257,7 +257,7 @@ static int amd_pmf_invoke_cmd_init(struct >>>> amd_pmf_dev *dev) >>>> return -ENODEV; >>>> } >>>> - dev_dbg(dev->dev, "Policy Binary size: %u bytes\n", >>>> dev->policy_sz); >>>> + dev_dbg(dev->dev, "Policy Binary size: %llu bytes\n", >>>> dev->policy_sz); >>>> memset(dev->shbuf, 0, dev->policy_sz); >>>> ta_sm = dev->shbuf; >>>> in = &ta_sm->pmf_input.init_table; >>>> @@ -512,9 +512,9 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev >>>> *dev) >>>> if (ret) >>>> goto error; >>>> - dev->policy_base = devm_ioremap(dev->dev, dev->policy_addr, >>>> dev->policy_sz); >>>> - if (!dev->policy_base) { >>>> - ret = -ENOMEM; >>>> + dev->policy_base = devm_ioremap_resource(dev->dev, dev->res); >>>> + if (IS_ERR(dev->policy_base)) { >>>> + ret = PTR_ERR(dev->policy_base); >>>> goto error; >>>> } >>>> >>> >
On 10/23/2024 10:14, Shyam Sundar S K wrote: > > > On 10/23/2024 20:20, Mario Limonciello wrote: >> On 10/23/2024 09:37, Shyam Sundar S K wrote: >>> >>> >>> On 10/23/2024 19:35, Mario Limonciello wrote: >>>> On 10/23/2024 01:32, Shyam Sundar S K wrote: >>>>> Use platform_get_resource() to fetch the memory resource instead of >>>>> acpi_walk_resources() and devm_ioremap_resource() for mapping the >>>>> resources. >>>>> >>>>> PS: We cannot use resource_size() here because it adds an extra byte >>>>> to round >>>>> off the size. In the case of PMF ResourceTemplate(), this rounding is >>>>> already handled within the _CRS. Using resource_size() would >>>>> increase the >>>>> resource size by 1, causing a mismatch with the length field and >>>>> leading >>>>> to issues. Therefore, simply use end-start of the ACPI resource to >>>>> obtain >>>>> the actual length. >>>>> >>>>> Co-developed-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> >>>>> Signed-off-by: Patil Rajesh Reddy <Patil.Reddy@amd.com> >>>>> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> >>>>> --- >>>>> drivers/platform/x86/amd/pmf/Kconfig | 1 + >>>>> drivers/platform/x86/amd/pmf/acpi.c | 46 >>>>> +++++++++++---------------- >>>>> drivers/platform/x86/amd/pmf/pmf.h | 6 ++-- >>>>> drivers/platform/x86/amd/pmf/tee-if.c | 8 ++--- >>>>> 4 files changed, 28 insertions(+), 33 deletions(-) >>>>> >>>>> diff --git a/drivers/platform/x86/amd/pmf/Kconfig >>>>> b/drivers/platform/x86/amd/pmf/Kconfig >>>>> index f4fa8bd8bda8..99d67cdbd91e 100644 >>>>> --- a/drivers/platform/x86/amd/pmf/Kconfig >>>>> +++ b/drivers/platform/x86/amd/pmf/Kconfig >>>>> @@ -11,6 +11,7 @@ config AMD_PMF >>>>> select ACPI_PLATFORM_PROFILE >>>>> depends on TEE && AMDTEE >>>>> depends on AMD_SFH_HID >>>>> + depends on HAS_IOMEM >>>>> help >>>>> This driver provides support for the AMD Platform Management >>>>> Framework. >>>>> The goal is to enhance end user experience by making AMD PCs >>>>> smarter, >>>>> diff --git a/drivers/platform/x86/amd/pmf/acpi.c >>>>> b/drivers/platform/x86/amd/pmf/acpi.c >>>>> index d5b496433d69..62f984fe40c6 100644 >>>>> --- a/drivers/platform/x86/amd/pmf/acpi.c >>>>> +++ b/drivers/platform/x86/amd/pmf/acpi.c >>>>> @@ -433,37 +433,29 @@ int apmf_install_handler(struct amd_pmf_dev >>>>> *pmf_dev) >>>>> return 0; >>>>> } >>>>> -static acpi_status apmf_walk_resources(struct acpi_resource *res, >>>>> void *data) >>>>> +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) >>>>> { >>>>> - struct amd_pmf_dev *dev = data; >>>>> + struct platform_device *pdev = to_platform_device(pmf_dev->dev); >>>>> - switch (res->type) { >>>>> - case ACPI_RESOURCE_TYPE_ADDRESS64: >>>>> - dev->policy_addr = res->data.address64.address.minimum; >>>>> - dev->policy_sz = res->data.address64.address.address_length; >>>>> - break; >>>>> - case ACPI_RESOURCE_TYPE_FIXED_MEMORY32: >>>>> - dev->policy_addr = res->data.fixed_memory32.address; >>>>> - dev->policy_sz = res->data.fixed_memory32.address_length; >>>>> - break; >>>>> - } >>>>> - >>>>> - if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ || >>>>> dev->policy_sz == 0) { >>>>> - pr_err("Incorrect Policy params, possibly a SBIOS bug\n"); >>>>> - return AE_ERROR; >>>>> + pmf_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>>>> + if (!pmf_dev->res) { >>>>> + dev_err(pmf_dev->dev, "Failed to get I/O memory >>>>> resource\n"); >>> >>> here ^^^^^^^ >>> >>>>> + return -EINVAL; >>>>> } >>>>> - return AE_OK; >>>>> -} >>>>> - >>>>> -int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) >>>>> -{ >>>>> - acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev); >>>>> - acpi_status status; >>>>> - >>>>> - status = acpi_walk_resources(ahandle, METHOD_NAME__CRS, >>>>> apmf_walk_resources, pmf_dev); >>>>> - if (ACPI_FAILURE(status)) { >>>>> - dev_dbg(pmf_dev->dev, "acpi_walk_resources failed :%d\n", >>>>> status); >>>>> + pmf_dev->policy_addr = pmf_dev->res->start; >>>>> + /* >>>>> + * We cannot use resource_size() here because it adds an extra >>>>> byte to round off the size. >>>>> + * In the case of PMF ResourceTemplate(), this rounding is >>>>> already handled within the _CRS. >>>>> + * Using resource_size() would increase the resource size by 1, >>>>> causing a mismatch with the >>>>> + * length field and leading to issues. Therefore, simply use >>>>> end-start of the ACPI resource >>>>> + * to obtain the actual length. >>>>> + */ >>>>> + pmf_dev->policy_sz = pmf_dev->res->end - pmf_dev->res->start; >>>>> + >>>>> + if (!pmf_dev->policy_addr || pmf_dev->policy_sz > >>>>> POLICY_BUF_MAX_SZ || >>>>> + pmf_dev->policy_sz == 0) { >>>>> + dev_err(pmf_dev->dev, "Incorrect policy params, possibly a >>>>> SBIOS bug\n"); >>>> >>>> This upgrades the previous message from debug to error. >>> >>> It is dev_err() even before this change. >>> >>>> >>>> TL;DR I feel this error should stay as dev_dbg() if no function checks >>>> are present for Smart PC. >>>> >>>> I don't think it's necessarily an error though. >>>> Smart PC checks are a bit different than the other checks. There >>>> isn't a check for a bit being set to indicate the function is >>>> supported. >>>> >>>> So if the BIOS has the declaration for the region but it's not >>>> populated it might not have a Smart PC policy and this shouldn't be >>>> reported as a BIOS bug. >>> >>> This should be included in the CPM package, and the BIOS team is >>> responsible for packaging a policy binary. >>> >>> From a driver design standpoint, the absence of the policy binary >>> should be treated as an error, as there's no reason for the BIOS to >>> advertise the Smart PC bits without providing the policy binary. >>> >>> Therefore, this should trigger a `dev_err()` and be considered a >>> BIOS bug. >>> >> >> OK I agree with this specific error, but I took a closer look at the >> bug associated with >> 03cea821b82cb ("platform/x86/amd: pmf: Decrease error message to debug") > > ah! but your comment was just inline to: > > dev_err(pmf_dev->dev, "Incorrect policy params, possibly a SBIOS bug\n"); > > So, I was thinking you are saying to downgrade this to dev_dbg() and > hence the above clarification. > > if the comment is for: > dev_err(pmf_dev->dev, "Failed to get I/O memory resource\n"); > > then Yes, I agree we should have dev_dbg() and I will respin a new > version. > It was originally for that line, but you corrected me. It looks that we reached the conclusion on the right line that should be fixed. Thanks!
diff --git a/drivers/platform/x86/amd/pmf/Kconfig b/drivers/platform/x86/amd/pmf/Kconfig index f4fa8bd8bda8..99d67cdbd91e 100644 --- a/drivers/platform/x86/amd/pmf/Kconfig +++ b/drivers/platform/x86/amd/pmf/Kconfig @@ -11,6 +11,7 @@ config AMD_PMF select ACPI_PLATFORM_PROFILE depends on TEE && AMDTEE depends on AMD_SFH_HID + depends on HAS_IOMEM help This driver provides support for the AMD Platform Management Framework. The goal is to enhance end user experience by making AMD PCs smarter, diff --git a/drivers/platform/x86/amd/pmf/acpi.c b/drivers/platform/x86/amd/pmf/acpi.c index d5b496433d69..62f984fe40c6 100644 --- a/drivers/platform/x86/amd/pmf/acpi.c +++ b/drivers/platform/x86/amd/pmf/acpi.c @@ -433,37 +433,29 @@ int apmf_install_handler(struct amd_pmf_dev *pmf_dev) return 0; } -static acpi_status apmf_walk_resources(struct acpi_resource *res, void *data) +int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) { - struct amd_pmf_dev *dev = data; + struct platform_device *pdev = to_platform_device(pmf_dev->dev); - switch (res->type) { - case ACPI_RESOURCE_TYPE_ADDRESS64: - dev->policy_addr = res->data.address64.address.minimum; - dev->policy_sz = res->data.address64.address.address_length; - break; - case ACPI_RESOURCE_TYPE_FIXED_MEMORY32: - dev->policy_addr = res->data.fixed_memory32.address; - dev->policy_sz = res->data.fixed_memory32.address_length; - break; - } - - if (!dev->policy_addr || dev->policy_sz > POLICY_BUF_MAX_SZ || dev->policy_sz == 0) { - pr_err("Incorrect Policy params, possibly a SBIOS bug\n"); - return AE_ERROR; + pmf_dev->res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!pmf_dev->res) { + dev_err(pmf_dev->dev, "Failed to get I/O memory resource\n"); + return -EINVAL; } - return AE_OK; -} - -int apmf_check_smart_pc(struct amd_pmf_dev *pmf_dev) -{ - acpi_handle ahandle = ACPI_HANDLE(pmf_dev->dev); - acpi_status status; - - status = acpi_walk_resources(ahandle, METHOD_NAME__CRS, apmf_walk_resources, pmf_dev); - if (ACPI_FAILURE(status)) { - dev_dbg(pmf_dev->dev, "acpi_walk_resources failed :%d\n", status); + pmf_dev->policy_addr = pmf_dev->res->start; + /* + * We cannot use resource_size() here because it adds an extra byte to round off the size. + * In the case of PMF ResourceTemplate(), this rounding is already handled within the _CRS. + * Using resource_size() would increase the resource size by 1, causing a mismatch with the + * length field and leading to issues. Therefore, simply use end-start of the ACPI resource + * to obtain the actual length. + */ + pmf_dev->policy_sz = pmf_dev->res->end - pmf_dev->res->start; + + if (!pmf_dev->policy_addr || pmf_dev->policy_sz > POLICY_BUF_MAX_SZ || + pmf_dev->policy_sz == 0) { + dev_err(pmf_dev->dev, "Incorrect policy params, possibly a SBIOS bug\n"); return -EINVAL; } diff --git a/drivers/platform/x86/amd/pmf/pmf.h b/drivers/platform/x86/amd/pmf/pmf.h index 8ce8816da9c1..a79808fda1d8 100644 --- a/drivers/platform/x86/amd/pmf/pmf.h +++ b/drivers/platform/x86/amd/pmf/pmf.h @@ -13,6 +13,7 @@ #include <linux/acpi.h> #include <linux/input.h> +#include <linux/platform_device.h> #include <linux/platform_profile.h> #define POLICY_BUF_MAX_SZ 0x4b000 @@ -355,19 +356,20 @@ struct amd_pmf_dev { /* Smart PC solution builder */ struct dentry *esbin; unsigned char *policy_buf; - u32 policy_sz; + resource_size_t policy_sz; struct tee_context *tee_ctx; struct tee_shm *fw_shm_pool; u32 session_id; void *shbuf; struct delayed_work pb_work; struct pmf_action_table *prev_data; - u64 policy_addr; + resource_size_t policy_addr; void __iomem *policy_base; bool smart_pc_enabled; u16 pmf_if_version; struct input_dev *pmf_idev; size_t mtable_size; + struct resource *res; }; struct apmf_sps_prop_granular_v2 { diff --git a/drivers/platform/x86/amd/pmf/tee-if.c b/drivers/platform/x86/amd/pmf/tee-if.c index 19c27b6e4666..555b8d6314e0 100644 --- a/drivers/platform/x86/amd/pmf/tee-if.c +++ b/drivers/platform/x86/amd/pmf/tee-if.c @@ -257,7 +257,7 @@ static int amd_pmf_invoke_cmd_init(struct amd_pmf_dev *dev) return -ENODEV; } - dev_dbg(dev->dev, "Policy Binary size: %u bytes\n", dev->policy_sz); + dev_dbg(dev->dev, "Policy Binary size: %llu bytes\n", dev->policy_sz); memset(dev->shbuf, 0, dev->policy_sz); ta_sm = dev->shbuf; in = &ta_sm->pmf_input.init_table; @@ -512,9 +512,9 @@ int amd_pmf_init_smart_pc(struct amd_pmf_dev *dev) if (ret) goto error; - dev->policy_base = devm_ioremap(dev->dev, dev->policy_addr, dev->policy_sz); - if (!dev->policy_base) { - ret = -ENOMEM; + dev->policy_base = devm_ioremap_resource(dev->dev, dev->res); + if (IS_ERR(dev->policy_base)) { + ret = PTR_ERR(dev->policy_base); goto error; }