On Thu, 10 Oct 2024, Mario Limonciello wrote: > From: Perry Yuan <Perry.Yuan@xxxxxxx> > > 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@xxxxxxx> > Co-developed-by: Mario Limonciello <mario.limonciello@xxxxxxx> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx> > --- > 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. > } > -- i.