| Message ID | 20241010193705.10362-7-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Add support for AMD hardware feedback interface | expand |
On Thu, 10 Oct 2024, Mario Limonciello wrote: > From: Perry Yuan <Perry.Yuan@amd.com> > > When `amd_hfi` driver is loaded, it will use PCCT subspace type 4 table > to retrieve the shared memory address which contains the CPU core ranking > table. This table includes a header that specifies the number of ranking > data entries to be parsed and rank each CPU core with the Performance and > Energy Efficiency capability as implemented by the CPU power management > firmware. > > Once the table has been parsed, each CPU is assigned a ranking score > within its class. Subsequently, when the scheduler selects cores, it > chooses from the ranking list based on the assigned scores in each class, > thereby ensuring the optimal selection of CPU cores according to their > predefined classifications and priorities. > > Signed-off-by: Perry Yuan <Perry.Yuan@amd.com> > Co-developed-by: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > v2: > * Rework amd_hfi_fill_metatadata to directly use structure instead of > pointer math. > --- > drivers/platform/x86/amd/hfi/hfi.c | 215 ++++++++++++++++++++++++++++- > 1 file changed, 212 insertions(+), 3 deletions(-) > > diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c > index da2e667107e8..10651399cf75 100644 > --- a/drivers/platform/x86/amd/hfi/hfi.c > +++ b/drivers/platform/x86/amd/hfi/hfi.c > @@ -18,22 +18,78 @@ > #include <linux/io.h> > #include <linux/kernel.h> > #include <linux/module.h> > +#include <linux/mailbox_client.h> > #include <linux/mutex.h> > +#include <linux/percpu-defs.h> > #include <linux/platform_device.h> > #include <linux/printk.h> > #include <linux/smp.h> > #include <linux/string.h> > +#include <linux/topology.h> > +#include <linux/workqueue.h> > + > +#include <asm/cpu_device_id.h> > + > +#include <acpi/pcc.h> > +#include <acpi/cppc_acpi.h> > > #define AMD_HFI_DRIVER "amd_hfi" > +#define AMD_HFI_MAILBOX_COUNT 1 > +#define AMD_HETERO_RANKING_TABLE_VER 2 > + > #define AMD_HETERO_CPUID_27 0x80000027 > + > static struct platform_device *device; > > +/** > + * struct amd_shmem_info - Shared memory table for AMD HFI > + * > + * @signature: The PCC signature. The signature of a subspace is computed by > + * a bitwise of the value 0x50434300 with the subspace ID. > + * @flags: Notify on completion > + * @length: Length of payload being transmitted including command field > + * @command: Command being sent over the subspace > + * @version_number: Version number of the table > + * @n_logical_processors: Number of logical processors > + * @n_capabilities: Number of ranking dimensions (performance, efficiency, etc) > + * @table_update_context: Command being sent over the subspace > + * @n_bitmaps: Number of 32-bit bitmaps to enumerate all the APIC IDs > + * This is based on the maximum APIC ID enumerated in the system > + * @reserved: 24 bit spare > + * @table_data: Bit Map(s) of enabled logical processors > + * Followed by the ranking data for each logical processor > + */ > +struct amd_shmem_info { > + struct acpi_pcct_ext_pcc_shared_memory header; > + u32 version_number :8, > + n_logical_processors :8, > + n_capabilities :8, > + table_update_context :8; > + u32 n_bitmaps :8, > + reserved :24; > + u32 table_data[]; > +} __packed; > + > struct amd_hfi_data { > const char *name; > struct device *dev; > struct mutex lock; > + > + /* PCCT table related*/ > + struct pcc_mbox_chan *pcc_chan; > + void __iomem *pcc_comm_addr; > + struct acpi_subtable_header *pcct_entry; > + struct amd_shmem_info *shmem; > }; > > +/** > + * struct amd_hfi_classes - HFI class capabilities per CPU > + * @perf: Performance capability > + * @eff: Power efficiency capability > + * > + * Capabilities of a logical processor in the ranking table. These capabilities > + * are unitless and specific to each HFI class. > + */ > struct amd_hfi_classes { > u32 perf; > u32 eff; > @@ -42,23 +98,105 @@ struct amd_hfi_classes { > /** > * struct amd_hfi_cpuinfo - HFI workload class info per CPU > * @cpu: cpu index > + * @apic_id: apic id of the current cpu > * @cpus: mask of cpus associated with amd_hfi_cpuinfo > * @class_index: workload class ID index > * @nr_class: max number of workload class supported > + * @ipcc_scores: ipcc scores for each class > * @amd_hfi_classes: current cpu workload class ranking data > * > * Parameters of a logical processor linked with hardware feedback class > */ > struct amd_hfi_cpuinfo { > int cpu; > + u32 apic_id; > cpumask_var_t cpus; > s16 class_index; > u8 nr_class; > + int *ipcc_scores; > struct amd_hfi_classes *amd_hfi_classes; > }; > > static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1}; > > +static int find_cpu_index_by_apicid(unsigned int target_apicid) > +{ > + int cpu_index; > + > + for_each_possible_cpu(cpu_index) { > + struct cpuinfo_x86 *info = &cpu_data(cpu_index); > + > + if (info->topo.apicid == target_apicid) { > + pr_debug("match APIC id %d for CPU index: %d", Missing \n > + info->topo.apicid, cpu_index); > + return cpu_index; > + } > + } > + > + return -ENODEV; > +} > + > +static int amd_hfi_fill_metadata(struct amd_hfi_data *amd_hfi_data) > +{ > + struct acpi_pcct_ext_pcc_slave *pcct_ext = > + (struct acpi_pcct_ext_pcc_slave *)amd_hfi_data->pcct_entry; > + void __iomem *pcc_comm_addr; > + > + pcc_comm_addr = acpi_os_ioremap(amd_hfi_data->pcc_chan->shmem_base_addr, > + amd_hfi_data->pcc_chan->shmem_size); > + if (!pcc_comm_addr) { > + pr_err("failed to ioremap PCC common region mem\n"); > + return -ENOMEM; > + } > + > + memcpy_fromio(amd_hfi_data->shmem, pcc_comm_addr, pcct_ext->length); > + iounmap(pcc_comm_addr); > + > + if (amd_hfi_data->shmem->header.signature != PCC_SIGNATURE) { > + pr_err("Invalid signature in shared memory\n"); > + return -EINVAL; > + } > + if (amd_hfi_data->shmem->version_number != AMD_HETERO_RANKING_TABLE_VER) { > + pr_err("Invalid veresion %d\n", amd_hfi_data->shmem->version_number); version > + return -EINVAL; > + } > + > + for (u32 i = 0; i < amd_hfi_data->shmem->n_bitmaps; i++) { > + u32 bitmap = amd_hfi_data->shmem->table_data[i]; > + > + for (u32 j = 0; j < BITS_PER_TYPE(u32); j++) { Are these u32 really the types you want to use for the loop vars, why? > + struct amd_hfi_cpuinfo *info; > + int apic_id = i * BITS_PER_TYPE(u32) + j; > + int cpu_index; > + > + if (!(bitmap & BIT(j))) > + continue; > + > + cpu_index = find_cpu_index_by_apicid(apic_id); > + if (cpu_index < 0) { > + pr_warn("APIC ID %d not found\n", apic_id); > + continue; > + } > + > + info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index); > + info->apic_id = apic_id; > + > + /* Fill the ranking data for each logical processor */ > + info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index); > + for (int k = 0; k < info->nr_class; k++) { unsigned int > + u32 *table = amd_hfi_data->shmem->table_data + > + amd_hfi_data->shmem->n_bitmaps + > + i * info->nr_class; > + > + info->amd_hfi_classes[k].eff = table[apic_id + 2 * k]; > + info->amd_hfi_classes[k].perf = table[apic_id + 2 * k + 1]; > + } > + } > + } > + > + return 0; > +} > + > static int amd_hfi_alloc_class_data(struct platform_device *pdev) > { > struct amd_hfi_cpuinfo *hfi_cpuinfo; > @@ -68,8 +206,7 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev) > > nr_class_id = cpuid_eax(AMD_HETERO_CPUID_27); > if (nr_class_id < 0 || nr_class_id > 255) { > - dev_warn(dev, "failed to get supported class number from CPUID %d\n", > - AMD_HETERO_CPUID_27); > + dev_warn(dev, "failed to get number of supported classes\n"); This message was added in the previous patch and now immediately changed. > return -EINVAL; > } > > @@ -79,7 +216,10 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev) > sizeof(struct amd_hfi_classes), GFP_KERNEL); > if (!hfi_cpuinfo->amd_hfi_classes) > return -ENOMEM; > - > + hfi_cpuinfo->ipcc_scores = devm_kcalloc(dev, nr_class_id, > + sizeof(int), GFP_KERNEL); > + if (!hfi_cpuinfo->ipcc_scores) > + return -ENOMEM; > hfi_cpuinfo->nr_class = nr_class_id; > } > > @@ -93,6 +233,70 @@ static void amd_hfi_remove(struct platform_device *pdev) > mutex_destroy(&dev->lock); > } > > +static int amd_hfi_metadata_parser(struct platform_device *pdev, > + struct amd_hfi_data *amd_hfi_data) > +{ > + struct acpi_pcct_ext_pcc_slave *pcct_ext; > + struct acpi_subtable_header *pcct_entry; > + struct mbox_chan *pcc_mbox_channels; > + struct acpi_table_header *pcct_tbl; > + struct pcc_mbox_chan *pcc_chan; > + acpi_status status; > + int ret; > + > + pcc_mbox_channels = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT, > + sizeof(*pcc_mbox_channels), GFP_KERNEL); > + if (!pcc_mbox_channels) { > + ret = -ENOMEM; > + goto out; Please return directly if there is nothing to rollback. > + } > + > + pcc_chan = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT, > + sizeof(*pcc_chan), GFP_KERNEL); > + if (!pcc_chan) { > + ret = -ENOMEM; > + goto out; Ditto. > + } > + > + status = acpi_get_table(ACPI_SIG_PCCT, 0, &pcct_tbl); > + if (ACPI_FAILURE(status) || !pcct_tbl) { > + ret = -ENODEV; > + goto out; Ditto. > + } > + > + /* get pointer to the first PCC subspace entry */ > + pcct_entry = (struct acpi_subtable_header *) ( > + (unsigned long)pcct_tbl + sizeof(struct acpi_table_pcct)); > + > + pcc_chan->mchan = &pcc_mbox_channels[0]; > + > + amd_hfi_data->pcc_chan = pcc_chan; > + amd_hfi_data->pcct_entry = pcct_entry; > + pcct_ext = (struct acpi_pcct_ext_pcc_slave *)pcct_entry; > + > + if (pcct_ext->length <= 0) { > + ret = -EINVAL; > + goto out; Ditto. > + } > + > + amd_hfi_data->shmem = devm_kmalloc(amd_hfi_data->dev, pcct_ext->length, GFP_KERNEL); Why kmalloc ? > + if (!amd_hfi_data->shmem) { > + ret = -ENOMEM; > + goto out; Return directly. > + } > + > + pcc_chan->shmem_base_addr = pcct_ext->base_address; > + pcc_chan->shmem_size = pcct_ext->length; > + > + /* parse the shared memory info from the pcct table */ > + ret = amd_hfi_fill_metadata(amd_hfi_data); > + > + acpi_put_table(pcct_tbl); > + > +out: > + return ret; > +} > + > static const struct acpi_device_id amd_hfi_platform_match[] = { > { "AMDI0104", 0}, > { } > @@ -121,6 +325,11 @@ static int amd_hfi_probe(struct platform_device *pdev) > if (ret) > goto out; This should do return ret; directly, not jump to out label which does nothing but return. > > + /* parse PCCT table */ > + ret = amd_hfi_metadata_parser(pdev, amd_hfi_data); > + if (ret) > + goto out; > + > out: > return ret; Might again be there for churn avoidance, otherwise, please consider: return amd_hfi_metadata_parser(pdev, amd_hfi_data); That goto out should again just return ret directly. > } >
On 10/14/2024 05:14, Ilpo Järvinen wrote: > On Thu, 10 Oct 2024, Mario Limonciello wrote: > >> From: Perry Yuan <Perry.Yuan@amd.com> >> >> When `amd_hfi` driver is loaded, it will use PCCT subspace type 4 table >> to retrieve the shared memory address which contains the CPU core ranking >> table. This table includes a header that specifies the number of ranking >> data entries to be parsed and rank each CPU core with the Performance and >> Energy Efficiency capability as implemented by the CPU power management >> firmware. >> >> Once the table has been parsed, each CPU is assigned a ranking score >> within its class. Subsequently, when the scheduler selects cores, it >> chooses from the ranking list based on the assigned scores in each class, >> thereby ensuring the optimal selection of CPU cores according to their >> predefined classifications and priorities. >> >> Signed-off-by: Perry Yuan <Perry.Yuan@amd.com> >> Co-developed-by: Mario Limonciello <mario.limonciello@amd.com> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> v2: >> * Rework amd_hfi_fill_metatadata to directly use structure instead of >> pointer math. >> --- >> drivers/platform/x86/amd/hfi/hfi.c | 215 ++++++++++++++++++++++++++++- >> 1 file changed, 212 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c >> index da2e667107e8..10651399cf75 100644 >> --- a/drivers/platform/x86/amd/hfi/hfi.c >> +++ b/drivers/platform/x86/amd/hfi/hfi.c >> @@ -18,22 +18,78 @@ >> #include <linux/io.h> >> #include <linux/kernel.h> >> #include <linux/module.h> >> +#include <linux/mailbox_client.h> >> #include <linux/mutex.h> >> +#include <linux/percpu-defs.h> >> #include <linux/platform_device.h> >> #include <linux/printk.h> >> #include <linux/smp.h> >> #include <linux/string.h> >> +#include <linux/topology.h> >> +#include <linux/workqueue.h> >> + >> +#include <asm/cpu_device_id.h> >> + >> +#include <acpi/pcc.h> >> +#include <acpi/cppc_acpi.h> >> >> #define AMD_HFI_DRIVER "amd_hfi" >> +#define AMD_HFI_MAILBOX_COUNT 1 >> +#define AMD_HETERO_RANKING_TABLE_VER 2 >> + >> #define AMD_HETERO_CPUID_27 0x80000027 >> + >> static struct platform_device *device; >> >> +/** >> + * struct amd_shmem_info - Shared memory table for AMD HFI >> + * >> + * @signature: The PCC signature. The signature of a subspace is computed by >> + * a bitwise of the value 0x50434300 with the subspace ID. >> + * @flags: Notify on completion >> + * @length: Length of payload being transmitted including command field >> + * @command: Command being sent over the subspace >> + * @version_number: Version number of the table >> + * @n_logical_processors: Number of logical processors >> + * @n_capabilities: Number of ranking dimensions (performance, efficiency, etc) >> + * @table_update_context: Command being sent over the subspace >> + * @n_bitmaps: Number of 32-bit bitmaps to enumerate all the APIC IDs >> + * This is based on the maximum APIC ID enumerated in the system >> + * @reserved: 24 bit spare >> + * @table_data: Bit Map(s) of enabled logical processors >> + * Followed by the ranking data for each logical processor >> + */ >> +struct amd_shmem_info { >> + struct acpi_pcct_ext_pcc_shared_memory header; >> + u32 version_number :8, >> + n_logical_processors :8, >> + n_capabilities :8, >> + table_update_context :8; >> + u32 n_bitmaps :8, >> + reserved :24; >> + u32 table_data[]; >> +} __packed; >> + >> struct amd_hfi_data { >> const char *name; >> struct device *dev; >> struct mutex lock; >> + >> + /* PCCT table related*/ >> + struct pcc_mbox_chan *pcc_chan; >> + void __iomem *pcc_comm_addr; >> + struct acpi_subtable_header *pcct_entry; >> + struct amd_shmem_info *shmem; >> }; >> >> +/** >> + * struct amd_hfi_classes - HFI class capabilities per CPU >> + * @perf: Performance capability >> + * @eff: Power efficiency capability >> + * >> + * Capabilities of a logical processor in the ranking table. These capabilities >> + * are unitless and specific to each HFI class. >> + */ >> struct amd_hfi_classes { >> u32 perf; >> u32 eff; >> @@ -42,23 +98,105 @@ struct amd_hfi_classes { >> /** >> * struct amd_hfi_cpuinfo - HFI workload class info per CPU >> * @cpu: cpu index >> + * @apic_id: apic id of the current cpu >> * @cpus: mask of cpus associated with amd_hfi_cpuinfo >> * @class_index: workload class ID index >> * @nr_class: max number of workload class supported >> + * @ipcc_scores: ipcc scores for each class >> * @amd_hfi_classes: current cpu workload class ranking data >> * >> * Parameters of a logical processor linked with hardware feedback class >> */ >> struct amd_hfi_cpuinfo { >> int cpu; >> + u32 apic_id; >> cpumask_var_t cpus; >> s16 class_index; >> u8 nr_class; >> + int *ipcc_scores; >> struct amd_hfi_classes *amd_hfi_classes; >> }; >> >> static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1}; >> >> +static int find_cpu_index_by_apicid(unsigned int target_apicid) >> +{ >> + int cpu_index; >> + >> + for_each_possible_cpu(cpu_index) { >> + struct cpuinfo_x86 *info = &cpu_data(cpu_index); >> + >> + if (info->topo.apicid == target_apicid) { >> + pr_debug("match APIC id %d for CPU index: %d", > > Missing \n Ack > >> + info->topo.apicid, cpu_index); >> + return cpu_index; >> + } >> + } >> + >> + return -ENODEV; >> +} >> + >> +static int amd_hfi_fill_metadata(struct amd_hfi_data *amd_hfi_data) >> +{ >> + struct acpi_pcct_ext_pcc_slave *pcct_ext = >> + (struct acpi_pcct_ext_pcc_slave *)amd_hfi_data->pcct_entry; >> + void __iomem *pcc_comm_addr; >> + >> + pcc_comm_addr = acpi_os_ioremap(amd_hfi_data->pcc_chan->shmem_base_addr, >> + amd_hfi_data->pcc_chan->shmem_size); >> + if (!pcc_comm_addr) { >> + pr_err("failed to ioremap PCC common region mem\n"); >> + return -ENOMEM; >> + } >> + >> + memcpy_fromio(amd_hfi_data->shmem, pcc_comm_addr, pcct_ext->length); >> + iounmap(pcc_comm_addr); >> + >> + if (amd_hfi_data->shmem->header.signature != PCC_SIGNATURE) { >> + pr_err("Invalid signature in shared memory\n"); >> + return -EINVAL; >> + } >> + if (amd_hfi_data->shmem->version_number != AMD_HETERO_RANKING_TABLE_VER) { >> + pr_err("Invalid veresion %d\n", amd_hfi_data->shmem->version_number); > > version Ack > >> + return -EINVAL; >> + } >> + >> + for (u32 i = 0; i < amd_hfi_data->shmem->n_bitmaps; i++) { >> + u32 bitmap = amd_hfi_data->shmem->table_data[i]; >> + >> + for (u32 j = 0; j < BITS_PER_TYPE(u32); j++) { > > Are these u32 really the types you want to use for the loop vars, why? I was going off the type of amd_hfi_data->shmem->n_bitmaps which is u32. In practice I think an unsigned int should be fine though too. > >> + struct amd_hfi_cpuinfo *info; >> + int apic_id = i * BITS_PER_TYPE(u32) + j; >> + int cpu_index; >> + >> + if (!(bitmap & BIT(j))) >> + continue; >> + >> + cpu_index = find_cpu_index_by_apicid(apic_id); >> + if (cpu_index < 0) { >> + pr_warn("APIC ID %d not found\n", apic_id); >> + continue; >> + } >> + >> + info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index); >> + info->apic_id = apic_id; >> + >> + /* Fill the ranking data for each logical processor */ >> + info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index); >> + for (int k = 0; k < info->nr_class; k++) { > > unsigned int > >> + u32 *table = amd_hfi_data->shmem->table_data + >> + amd_hfi_data->shmem->n_bitmaps + >> + i * info->nr_class; >> + >> + info->amd_hfi_classes[k].eff = table[apic_id + 2 * k]; >> + info->amd_hfi_classes[k].perf = table[apic_id + 2 * k + 1]; >> + } >> + } >> + } >> + >> + return 0; >> +} >> + >> static int amd_hfi_alloc_class_data(struct platform_device *pdev) >> { >> struct amd_hfi_cpuinfo *hfi_cpuinfo; >> @@ -68,8 +206,7 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev) >> >> nr_class_id = cpuid_eax(AMD_HETERO_CPUID_27); >> if (nr_class_id < 0 || nr_class_id > 255) { >> - dev_warn(dev, "failed to get supported class number from CPUID %d\n", >> - AMD_HETERO_CPUID_27); >> + dev_warn(dev, "failed to get number of supported classes\n"); > > This message was added in the previous patch and now immediately changed. Will drop this change. > >> return -EINVAL; >> } >> >> @@ -79,7 +216,10 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev) >> sizeof(struct amd_hfi_classes), GFP_KERNEL); >> if (!hfi_cpuinfo->amd_hfi_classes) >> return -ENOMEM; >> - >> + hfi_cpuinfo->ipcc_scores = devm_kcalloc(dev, nr_class_id, >> + sizeof(int), GFP_KERNEL); >> + if (!hfi_cpuinfo->ipcc_scores) >> + return -ENOMEM; >> hfi_cpuinfo->nr_class = nr_class_id; >> } >> >> @@ -93,6 +233,70 @@ static void amd_hfi_remove(struct platform_device *pdev) >> mutex_destroy(&dev->lock); >> } >> >> +static int amd_hfi_metadata_parser(struct platform_device *pdev, >> + struct amd_hfi_data *amd_hfi_data) >> +{ >> + struct acpi_pcct_ext_pcc_slave *pcct_ext; >> + struct acpi_subtable_header *pcct_entry; >> + struct mbox_chan *pcc_mbox_channels; >> + struct acpi_table_header *pcct_tbl; >> + struct pcc_mbox_chan *pcc_chan; >> + acpi_status status; >> + int ret; >> + >> + pcc_mbox_channels = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT, >> + sizeof(*pcc_mbox_channels), GFP_KERNEL); >> + if (!pcc_mbox_channels) { >> + ret = -ENOMEM; >> + goto out; > > Please return directly if there is nothing to rollback. Ack > >> + } >> + >> + pcc_chan = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT, >> + sizeof(*pcc_chan), GFP_KERNEL); >> + if (!pcc_chan) { >> + ret = -ENOMEM; >> + goto out; > > Ditto. > >> + } >> + >> + status = acpi_get_table(ACPI_SIG_PCCT, 0, &pcct_tbl); >> + if (ACPI_FAILURE(status) || !pcct_tbl) { >> + ret = -ENODEV; >> + goto out; > > Ditto. > >> + } >> + >> + /* get pointer to the first PCC subspace entry */ >> + pcct_entry = (struct acpi_subtable_header *) ( >> + (unsigned long)pcct_tbl + sizeof(struct acpi_table_pcct)); >> + >> + pcc_chan->mchan = &pcc_mbox_channels[0]; >> + >> + amd_hfi_data->pcc_chan = pcc_chan; >> + amd_hfi_data->pcct_entry = pcct_entry; >> + pcct_ext = (struct acpi_pcct_ext_pcc_slave *)pcct_entry; >> + >> + if (pcct_ext->length <= 0) { >> + ret = -EINVAL; >> + goto out; > > Ditto. > >> + } >> + >> + amd_hfi_data->shmem = devm_kmalloc(amd_hfi_data->dev, pcct_ext->length, GFP_KERNEL); > > Why kmalloc ? > >> + if (!amd_hfi_data->shmem) { >> + ret = -ENOMEM; >> + goto out; > > Return directly. > >> + } >> + >> + pcc_chan->shmem_base_addr = pcct_ext->base_address; >> + pcc_chan->shmem_size = pcct_ext->length; >> + >> + /* parse the shared memory info from the pcct table */ >> + ret = amd_hfi_fill_metadata(amd_hfi_data); >> + >> + acpi_put_table(pcct_tbl); >> + >> +out: >> + return ret; >> +} >> + >> static const struct acpi_device_id amd_hfi_platform_match[] = { >> { "AMDI0104", 0}, >> { } >> @@ -121,6 +325,11 @@ static int amd_hfi_probe(struct platform_device *pdev) >> if (ret) >> goto out; > > This should do return ret; directly, not jump to out label which does > nothing but return. > >> >> + /* parse PCCT table */ >> + ret = amd_hfi_metadata_parser(pdev, amd_hfi_data); >> + if (ret) >> + goto out; >> + >> out: >> return ret; > > Might again be there for churn avoidance, otherwise, please consider: > > return amd_hfi_metadata_parser(pdev, amd_hfi_data); > > That goto out should again just return ret directly. > >> } >> >
diff --git a/drivers/platform/x86/amd/hfi/hfi.c b/drivers/platform/x86/amd/hfi/hfi.c index da2e667107e8..10651399cf75 100644 --- a/drivers/platform/x86/amd/hfi/hfi.c +++ b/drivers/platform/x86/amd/hfi/hfi.c @@ -18,22 +18,78 @@ #include <linux/io.h> #include <linux/kernel.h> #include <linux/module.h> +#include <linux/mailbox_client.h> #include <linux/mutex.h> +#include <linux/percpu-defs.h> #include <linux/platform_device.h> #include <linux/printk.h> #include <linux/smp.h> #include <linux/string.h> +#include <linux/topology.h> +#include <linux/workqueue.h> + +#include <asm/cpu_device_id.h> + +#include <acpi/pcc.h> +#include <acpi/cppc_acpi.h> #define AMD_HFI_DRIVER "amd_hfi" +#define AMD_HFI_MAILBOX_COUNT 1 +#define AMD_HETERO_RANKING_TABLE_VER 2 + #define AMD_HETERO_CPUID_27 0x80000027 + static struct platform_device *device; +/** + * struct amd_shmem_info - Shared memory table for AMD HFI + * + * @signature: The PCC signature. The signature of a subspace is computed by + * a bitwise of the value 0x50434300 with the subspace ID. + * @flags: Notify on completion + * @length: Length of payload being transmitted including command field + * @command: Command being sent over the subspace + * @version_number: Version number of the table + * @n_logical_processors: Number of logical processors + * @n_capabilities: Number of ranking dimensions (performance, efficiency, etc) + * @table_update_context: Command being sent over the subspace + * @n_bitmaps: Number of 32-bit bitmaps to enumerate all the APIC IDs + * This is based on the maximum APIC ID enumerated in the system + * @reserved: 24 bit spare + * @table_data: Bit Map(s) of enabled logical processors + * Followed by the ranking data for each logical processor + */ +struct amd_shmem_info { + struct acpi_pcct_ext_pcc_shared_memory header; + u32 version_number :8, + n_logical_processors :8, + n_capabilities :8, + table_update_context :8; + u32 n_bitmaps :8, + reserved :24; + u32 table_data[]; +} __packed; + struct amd_hfi_data { const char *name; struct device *dev; struct mutex lock; + + /* PCCT table related*/ + struct pcc_mbox_chan *pcc_chan; + void __iomem *pcc_comm_addr; + struct acpi_subtable_header *pcct_entry; + struct amd_shmem_info *shmem; }; +/** + * struct amd_hfi_classes - HFI class capabilities per CPU + * @perf: Performance capability + * @eff: Power efficiency capability + * + * Capabilities of a logical processor in the ranking table. These capabilities + * are unitless and specific to each HFI class. + */ struct amd_hfi_classes { u32 perf; u32 eff; @@ -42,23 +98,105 @@ struct amd_hfi_classes { /** * struct amd_hfi_cpuinfo - HFI workload class info per CPU * @cpu: cpu index + * @apic_id: apic id of the current cpu * @cpus: mask of cpus associated with amd_hfi_cpuinfo * @class_index: workload class ID index * @nr_class: max number of workload class supported + * @ipcc_scores: ipcc scores for each class * @amd_hfi_classes: current cpu workload class ranking data * * Parameters of a logical processor linked with hardware feedback class */ struct amd_hfi_cpuinfo { int cpu; + u32 apic_id; cpumask_var_t cpus; s16 class_index; u8 nr_class; + int *ipcc_scores; struct amd_hfi_classes *amd_hfi_classes; }; static DEFINE_PER_CPU(struct amd_hfi_cpuinfo, amd_hfi_cpuinfo) = {.class_index = -1}; +static int find_cpu_index_by_apicid(unsigned int target_apicid) +{ + int cpu_index; + + for_each_possible_cpu(cpu_index) { + struct cpuinfo_x86 *info = &cpu_data(cpu_index); + + if (info->topo.apicid == target_apicid) { + pr_debug("match APIC id %d for CPU index: %d", + info->topo.apicid, cpu_index); + return cpu_index; + } + } + + return -ENODEV; +} + +static int amd_hfi_fill_metadata(struct amd_hfi_data *amd_hfi_data) +{ + struct acpi_pcct_ext_pcc_slave *pcct_ext = + (struct acpi_pcct_ext_pcc_slave *)amd_hfi_data->pcct_entry; + void __iomem *pcc_comm_addr; + + pcc_comm_addr = acpi_os_ioremap(amd_hfi_data->pcc_chan->shmem_base_addr, + amd_hfi_data->pcc_chan->shmem_size); + if (!pcc_comm_addr) { + pr_err("failed to ioremap PCC common region mem\n"); + return -ENOMEM; + } + + memcpy_fromio(amd_hfi_data->shmem, pcc_comm_addr, pcct_ext->length); + iounmap(pcc_comm_addr); + + if (amd_hfi_data->shmem->header.signature != PCC_SIGNATURE) { + pr_err("Invalid signature in shared memory\n"); + return -EINVAL; + } + if (amd_hfi_data->shmem->version_number != AMD_HETERO_RANKING_TABLE_VER) { + pr_err("Invalid veresion %d\n", amd_hfi_data->shmem->version_number); + return -EINVAL; + } + + for (u32 i = 0; i < amd_hfi_data->shmem->n_bitmaps; i++) { + u32 bitmap = amd_hfi_data->shmem->table_data[i]; + + for (u32 j = 0; j < BITS_PER_TYPE(u32); j++) { + struct amd_hfi_cpuinfo *info; + int apic_id = i * BITS_PER_TYPE(u32) + j; + int cpu_index; + + if (!(bitmap & BIT(j))) + continue; + + cpu_index = find_cpu_index_by_apicid(apic_id); + if (cpu_index < 0) { + pr_warn("APIC ID %d not found\n", apic_id); + continue; + } + + info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index); + info->apic_id = apic_id; + + /* Fill the ranking data for each logical processor */ + info = per_cpu_ptr(&amd_hfi_cpuinfo, cpu_index); + for (int k = 0; k < info->nr_class; k++) { + u32 *table = amd_hfi_data->shmem->table_data + + amd_hfi_data->shmem->n_bitmaps + + i * info->nr_class; + + info->amd_hfi_classes[k].eff = table[apic_id + 2 * k]; + info->amd_hfi_classes[k].perf = table[apic_id + 2 * k + 1]; + } + } + } + + return 0; +} + static int amd_hfi_alloc_class_data(struct platform_device *pdev) { struct amd_hfi_cpuinfo *hfi_cpuinfo; @@ -68,8 +206,7 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev) nr_class_id = cpuid_eax(AMD_HETERO_CPUID_27); if (nr_class_id < 0 || nr_class_id > 255) { - dev_warn(dev, "failed to get supported class number from CPUID %d\n", - AMD_HETERO_CPUID_27); + dev_warn(dev, "failed to get number of supported classes\n"); return -EINVAL; } @@ -79,7 +216,10 @@ static int amd_hfi_alloc_class_data(struct platform_device *pdev) sizeof(struct amd_hfi_classes), GFP_KERNEL); if (!hfi_cpuinfo->amd_hfi_classes) return -ENOMEM; - + hfi_cpuinfo->ipcc_scores = devm_kcalloc(dev, nr_class_id, + sizeof(int), GFP_KERNEL); + if (!hfi_cpuinfo->ipcc_scores) + return -ENOMEM; hfi_cpuinfo->nr_class = nr_class_id; } @@ -93,6 +233,70 @@ static void amd_hfi_remove(struct platform_device *pdev) mutex_destroy(&dev->lock); } +static int amd_hfi_metadata_parser(struct platform_device *pdev, + struct amd_hfi_data *amd_hfi_data) +{ + struct acpi_pcct_ext_pcc_slave *pcct_ext; + struct acpi_subtable_header *pcct_entry; + struct mbox_chan *pcc_mbox_channels; + struct acpi_table_header *pcct_tbl; + struct pcc_mbox_chan *pcc_chan; + acpi_status status; + int ret; + + pcc_mbox_channels = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT, + sizeof(*pcc_mbox_channels), GFP_KERNEL); + if (!pcc_mbox_channels) { + ret = -ENOMEM; + goto out; + } + + pcc_chan = devm_kcalloc(&pdev->dev, AMD_HFI_MAILBOX_COUNT, + sizeof(*pcc_chan), GFP_KERNEL); + if (!pcc_chan) { + ret = -ENOMEM; + goto out; + } + + status = acpi_get_table(ACPI_SIG_PCCT, 0, &pcct_tbl); + if (ACPI_FAILURE(status) || !pcct_tbl) { + ret = -ENODEV; + goto out; + } + + /* get pointer to the first PCC subspace entry */ + pcct_entry = (struct acpi_subtable_header *) ( + (unsigned long)pcct_tbl + sizeof(struct acpi_table_pcct)); + + pcc_chan->mchan = &pcc_mbox_channels[0]; + + amd_hfi_data->pcc_chan = pcc_chan; + amd_hfi_data->pcct_entry = pcct_entry; + pcct_ext = (struct acpi_pcct_ext_pcc_slave *)pcct_entry; + + if (pcct_ext->length <= 0) { + ret = -EINVAL; + goto out; + } + + amd_hfi_data->shmem = devm_kmalloc(amd_hfi_data->dev, pcct_ext->length, GFP_KERNEL); + if (!amd_hfi_data->shmem) { + ret = -ENOMEM; + goto out; + } + + pcc_chan->shmem_base_addr = pcct_ext->base_address; + pcc_chan->shmem_size = pcct_ext->length; + + /* parse the shared memory info from the pcct table */ + ret = amd_hfi_fill_metadata(amd_hfi_data); + + acpi_put_table(pcct_tbl); + +out: + return ret; +} + static const struct acpi_device_id amd_hfi_platform_match[] = { { "AMDI0104", 0}, { } @@ -121,6 +325,11 @@ static int amd_hfi_probe(struct platform_device *pdev) if (ret) goto out; + /* parse PCCT table */ + ret = amd_hfi_metadata_parser(pdev, amd_hfi_data); + if (ret) + goto out; + out: return ret; }