lkml.org 
[lkml]   [2008]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 11/24] x86/oprofile: Add IBS support for AMD CPUs, model specific code

* Robert Richter <robert.richter@amd.com> wrote:

> @@ -132,6 +214,65 @@ static int op_amd_check_ctrs(struct pt_regs * const regs,
> }
> }
>
> + /*If AMD and IBS is available */
> + if (ibs_allowed && ibs_config.fetch_enabled) {
> + rdmsr(MSR_AMD64_IBSFETCHCTL, low, high);
> + if (high & IBS_FETCH_VALID_BIT) {
> + ibs_fetch.ibs_fetch_ctl_high = high;
> + ibs_fetch.ibs_fetch_ctl_low = low;
> + rdmsr(MSR_AMD64_IBSFETCHLINAD, low, high);
> + ibs_fetch.ibs_fetch_lin_addr_high = high;
> + ibs_fetch.ibs_fetch_lin_addr_low = low;
> + rdmsr(MSR_AMD64_IBSFETCHPHYSAD, low, high);
> + ibs_fetch.ibs_fetch_phys_addr_high = high;
> + ibs_fetch.ibs_fetch_phys_addr_low = low;
> +
> + oprofile_add_ibs_sample(regs,
> + (unsigned int *)&ibs_fetch,
> + IBS_FETCH_BEGIN);

please move this innermost code into a helper function. That will help
eliminate line 80 artifacts as well.

> + if (ibs_allowed && ibs_config.op_enabled) {
> + rdmsr(MSR_AMD64_IBSOPCTL, low, high);
> + if (low & IBS_OP_VALID_BIT) {
> + rdmsr(MSR_AMD64_IBSOPRIP, low, high);
> + ibs_op.ibs_op_rip_low = low;
> + ibs_op.ibs_op_rip_high = high;
> + rdmsr(MSR_AMD64_IBSOPDATA, low, high);
> + ibs_op.ibs_op_data1_low = low;
> + ibs_op.ibs_op_data1_high = high;
> + rdmsr(MSR_AMD64_IBSOPDATA2, low, high);
> + ibs_op.ibs_op_data2_low = low;
> + ibs_op.ibs_op_data2_high = high;
> + rdmsr(MSR_AMD64_IBSOPDATA3, low, high);
> + ibs_op.ibs_op_data3_low = low;
> + ibs_op.ibs_op_data3_high = high;
> + rdmsr(MSR_AMD64_IBSDCLINAD, low, high);
> + ibs_op.ibs_dc_linear_low = low;
> + ibs_op.ibs_dc_linear_high = high;
> + rdmsr(MSR_AMD64_IBSDCPHYSAD, low, high);
> + ibs_op.ibs_dc_phys_low = low;
> + ibs_op.ibs_dc_phys_high = high;
> +
> + /* reenable the IRQ */
> + oprofile_add_ibs_sample(regs,
> + (unsigned int *)&ibs_op,
> + IBS_OP_BEGIN);
> + rdmsr(MSR_AMD64_IBSOPCTL, low, high);
> + low &= ~(IBS_OP_VALID_BIT);
> + low |= IBS_OP_ENABLE;
> + wrmsr(MSR_AMD64_IBSOPCTL, low, high);
> + }
> + }

ditto.

> + /**** Be sure to run loop until NULL is returned to
> + decrement reference count on any pci_dev structures
> + returned ****/

please use standard comment style:

/*
* Multi-line ..........
* ....................... comment:
*/

> + while ((gh_device = pci_get_device(PCI_VENDOR_ID_AMD,
> + PCI_DEVICE_ID_AMD_10H_NB_MISC, gh_device))
> + != NULL) {
> + /* This code may change if we can find a proper
> + * way to get at the PCI extended config space */

ditto.

> + pci_write_config_dword(
> + gh_device, IBS_LVT_OFFSET_PCI,
> + (vector | IBS_CTL_LVT_OFFSET_VALID_BIT));

perhaps use a helper function to reduce col 80 artifacts.

> +/*
> + * unitialize the APIC for the IBS interrupts if needed on AMD Family10h
> + * rev B0 and later */

comment style.

> +static void setup_ibs_files(struct super_block *sb, struct dentry *root)
> +{
> + char buf[12];

magic limit.

> + struct dentry *dir;
> +
> + if (!ibs_allowed)
> + return;
> +
> + /* setup some reasonable defaults */
> + ibs_config.max_cnt_fetch = 250000;
> + ibs_config.fetch_enabled = 0;
> + ibs_config.max_cnt_op = 250000;
> + ibs_config.op_enabled = 0;
> + ibs_config.dispatched_ops = 1;
> + snprintf(buf, sizeof(buf), "ibs_fetch");
> + dir = oprofilefs_mkdir(sb, root, buf);

you could make use of a small style trick here: use a newline before
this statement, to make blocks of code stand out in a clearer way. The
construction of 'buf' is finished after the snprintf above, then comes
the next step, to create the oprofilefs entries.

Ingo


\
 
 \ /
  Last update: 2008-07-26 12:07    [W:1.826 / U:0.016 seconds]
©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site