[vivid][PATCH] UBUNTU: [Config] CONFIG_PCIEASPM_DEBUG=y

[Andy Whitcroft]

On Sun, Dec 07, 2014 at 11:18:28PM +0000, Colin Ian King wrote:
> On 05/12/14 19:51, Chris J Arges wrote:
> > Here are the device attributes that get added and their permissions:
> > 
> > static DEVICE_ATTR(link_state, 0644, link_state_show, link_state_store);
> > static DEVICE_ATTR(clk_ctl, 0644, clk_ctl_show, clk_ctl_store);
> > 
> > These are only added if link_state->{aspm_support,clkpm_capable} are
> > true respectively.
> > 
> > So this does allow for storing those variables. And looking through the
> > kernel I don't see many examples of this pattern; in addition the
> > parameter checking seems a bit weak:
> > 
> > static ssize_t link_state_store(struct device *dev,
> >                 struct device_attribute *attr,
> >                 const char *buf,
> >                 size_t n)
> > {
> >         struct pci_dev *pdev = to_pci_dev(dev);
> >         struct pcie_link_state *link, *root = pdev->link_state->root;
> >         u32 val = buf[0] - '0', state = 0;

Ok, I've checked that buf cannot be NULL, and indeed count cannot be 0
...

> >         if (aspm_disabled)
> >                 return -EPERM;
> >         if (n < 1 || val > 3)
> >                 return -EINVAL;

This here actually checks count (n), and the converted value is between
0 and 3, which are the valid values.

> > static ssize_t clk_ctl_store(struct device *dev,
> >                 struct device_attribute *attr,
> >                 const char *buf,
> >                 size_t n)
> > {
> >         struct pci_dev *pdev = to_pci_dev(dev);
> >         int state;
> > 
> >         if (n < 1)
> >                 return -EINVAL;
> >         state = buf[0]-'0';
> > ...
> > 

Here the state is actually used as below:

        pcie_set_clkpm_nocheck(pdev->link_state, !!state);

ie, it is either true (!0) or false (0), and is "validated" in some sense.

I have also confirmed those are the only two new interfaces.  So I think
we should be safe here.

-apw