| Message ID | 20200518163523.1225643-4-sean.v.kelley@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: Add basic Compute eXpress Link DVSEC decode | expand |
On Mon, May 18, 2020 at 09:35:23AM -0700, Sean V Kelley wrote: > With these helpers, a device driver can enable/disable access to > CXL.mem and CXL.cache. Note that the device driver is responsible for > managing the memory area. > > Signed-off-by: Sean V Kelley <sean.v.kelley@linux.intel.com> > --- > drivers/pci/cxl.c | 93 ++++++++++++++++++++++++++++++++++++++++++++--- > drivers/pci/pci.h | 8 ++++ > 2 files changed, 96 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/cxl.c b/drivers/pci/cxl.c > index 4437ca69ad33..0d0a1b82ea98 100644 > --- a/drivers/pci/cxl.c > +++ b/drivers/pci/cxl.c > @@ -24,6 +24,88 @@ > #define PCI_CXL_HDM_COUNT(reg) (((reg) & (3 << 4)) >> 4) > #define PCI_CXL_VIRAL BIT(14) > > +#define PCI_CXL_CONFIG_LOCK BIT(0) > + > +static void pci_cxl_unlock(struct pci_dev *dev) > +{ > + int pos = dev->cxl_cap; > + u16 lock; > + > + pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); > + lock &= ~PCI_CXL_CONFIG_LOCK; > + pci_write_config_word(dev, pos + PCI_CXL_LOCK, lock); > +} > + > +static void pci_cxl_lock(struct pci_dev *dev) > +{ > + int pos = dev->cxl_cap; > + u16 lock; > + > + pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); > + lock |= PCI_CXL_CONFIG_LOCK; > + pci_write_config_word(dev, pos + PCI_CXL_LOCK, lock); > +} Maybe a tiny comment about what PCI_CXL_CONFIG_LOCK does? I guess these functions don't enforce mutual exclusion (as I first expected a "lock" function to do); they're simply telling the device to accept CTRL changes or something? > +static int pci_cxl_enable_disable_feature(struct pci_dev *dev, int enable, > + u16 feature) > +{ > + int pos = dev->cxl_cap; > + int ret; > + u16 reg; > + > + if (!dev->cxl_cap) > + return -EINVAL; "pos" "pos" is the typical name for things like this, but sometimes I think the code would be more descriptive if we used things like "cxl", "aer", etc. > + > + /* Only for PCIe */ > + if (!pci_is_pcie(dev)) > + return -EINVAL; Probably not necessary here. We already checked it in pci_cxi_init(). > + /* Only for Device 0 Function 0, Root Complex Integrated Endpoints */ > + if (dev->devfn != 0 || (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END)) > + return -EINVAL; > + > + pci_cxl_unlock(dev); > + ret = pci_read_config_word(dev, pos + PCI_CXL_CTRL, ®); > + if (ret) > + goto lock; > + > + if (enable) > + reg |= feature; > + else > + reg &= ~feature; > + > + ret = pci_write_config_word(dev, pos + PCI_CXL_CTRL, reg); > + > +lock: > + pci_cxl_lock(dev); > + > + return ret; > +} > + > +int pci_cxl_mem_enable(struct pci_dev *dev) > +{ > + return pci_cxl_enable_disable_feature(dev, true, PCI_CXL_MEM); > +} > +EXPORT_SYMBOL_GPL(pci_cxl_mem_enable); > + > +void pci_cxl_mem_disable(struct pci_dev *dev) > +{ > + pci_cxl_enable_disable_feature(dev, false, PCI_CXL_MEM); > +} > +EXPORT_SYMBOL_GPL(pci_cxl_mem_disable); > + > +int pci_cxl_cache_enable(struct pci_dev *dev) > +{ > + return pci_cxl_enable_disable_feature(dev, true, PCI_CXL_CACHE); > +} > +EXPORT_SYMBOL_GPL(pci_cxl_cache_enable); > + > +void pci_cxl_cache_disable(struct pci_dev *dev) > +{ > + pci_cxl_enable_disable_feature(dev, false, PCI_CXL_CACHE); > +} > +EXPORT_SYMBOL_GPL(pci_cxl_cache_disable); > + > /* > * pci_find_cxl_capability - Identify and return offset to Vendor-Specific > * capabilities. > @@ -73,11 +155,6 @@ void pci_cxl_init(struct pci_dev *dev) > dev->cxl_cap = pos; > > pci_read_config_word(dev, pos + PCI_CXL_CAP, &cap); > - pci_read_config_word(dev, pos + PCI_CXL_CTRL, &ctrl); > - pci_read_config_word(dev, pos + PCI_CXL_STS, &status); > - pci_read_config_word(dev, pos + PCI_CXL_CTRL2, &ctrl2); > - pci_read_config_word(dev, pos + PCI_CXL_STS2, &status2); > - pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); > > pci_info(dev, "CXL: Cache%c IO%c Mem%c Viral%c HDMCount %d\n", > FLAG(cap, PCI_CXL_CACHE), > @@ -86,6 +163,12 @@ void pci_cxl_init(struct pci_dev *dev) > FLAG(cap, PCI_CXL_VIRAL), > PCI_CXL_HDM_COUNT(cap)); > > + pci_read_config_word(dev, pos + PCI_CXL_CTRL, &ctrl); > + pci_read_config_word(dev, pos + PCI_CXL_STS, &status); > + pci_read_config_word(dev, pos + PCI_CXL_CTRL2, &ctrl2); > + pci_read_config_word(dev, pos + PCI_CXL_STS2, &status2); > + pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); This looks like it's just moving these reads around and isn't actually related to this patch. Put them where you want them in the first place so the move doesn't clutter this patch. > pci_info(dev, "CXL: cap ctrl status ctrl2 status2 lock\n"); > pci_info(dev, "CXL: %04x %04x %04x %04x %04x %04x\n", > cap, ctrl, status, ctrl2, status2, lock); > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > index d9905e2dee95..6336e16565ac 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -472,8 +472,16 @@ static inline void pci_restore_ats_state(struct pci_dev *dev) { } > #ifdef CONFIG_PCI_CXL > /* Compute eXpress Link */ > void pci_cxl_init(struct pci_dev *dev); > +int pci_cxl_mem_enable(struct pci_dev *dev); > +void pci_cxl_mem_disable(struct pci_dev *dev); > +int pci_cxl_cache_enable(struct pci_dev *dev); > +void pci_cxl_cache_disable(struct pci_dev *dev); > #else > static inline void pci_cxl_init(struct pci_dev *dev) { } > +static inline int pci_cxl_mem_enable(struct pci_dev *dev) {} > +static inline void pci_cxl_mem_disable(struct pci_dev *dev) {} > +static inline int pci_cxl_cache_enable(struct pci_dev *dev) {} > +static inline void pci_cxl_cache_disable(struct pci_dev *dev) {} The stubs in this file aren't completely consistent, but you can be at least locally consistent by including a space between the "{ }". > #endif > > #ifdef CONFIG_PCI_PRI > -- > 2.26.2 >
Hi Sean,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on pci/next]
[also build test WARNING on v5.7-rc6 next-20200518]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Sean-V-Kelley/PCI-Add-basic-Compute-eXpress-Link-DVSEC-decode/20200519-004029
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: arc-defconfig (attached as .config)
compiler: arc-elf-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
In file included from drivers/pci/of.c:16:
drivers/pci/pci.h: In function 'pci_cxl_mem_enable':
>> drivers/pci/pci.h:481:45: warning: no return statement in function returning non-void [-Wreturn-type]
481 | static inline int pci_cxl_mem_enable(struct pci_dev *dev) {}
| ^~~~~~~
drivers/pci/pci.h: In function 'pci_cxl_cache_enable':
drivers/pci/pci.h:483:47: warning: no return statement in function returning non-void [-Wreturn-type]
483 | static inline int pci_cxl_cache_enable(struct pci_dev *dev) {}
| ^~~~~~~
vim +481 drivers/pci/pci.h
471
472 #ifdef CONFIG_PCI_CXL
473 /* Compute eXpress Link */
474 void pci_cxl_init(struct pci_dev *dev);
475 int pci_cxl_mem_enable(struct pci_dev *dev);
476 void pci_cxl_mem_disable(struct pci_dev *dev);
477 int pci_cxl_cache_enable(struct pci_dev *dev);
478 void pci_cxl_cache_disable(struct pci_dev *dev);
479 #else
480 static inline void pci_cxl_init(struct pci_dev *dev) { }
> 481 static inline int pci_cxl_mem_enable(struct pci_dev *dev) {}
482 static inline void pci_cxl_mem_disable(struct pci_dev *dev) {}
483 static inline int pci_cxl_cache_enable(struct pci_dev *dev) {}
484 static inline void pci_cxl_cache_disable(struct pci_dev *dev) {}
485 #endif
486
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Sean,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on pci/next]
[also build test WARNING on v5.7-rc6 next-20200518]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Sean-V-Kelley/PCI-Add-basic-Compute-eXpress-Link-DVSEC-decode/20200519-004029
base: https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
In file included from drivers/pci/hotplug/pci_hotplug_core.c:32:
drivers/pci/hotplug/../pci.h: In function 'pci_cxl_mem_enable':
>> drivers/pci/hotplug/../pci.h:481:45: warning: no return statement in function returning non-void [-Wreturn-type]
481 | static inline int pci_cxl_mem_enable(struct pci_dev *dev) {}
| ^~~~~~~
drivers/pci/hotplug/../pci.h: In function 'pci_cxl_cache_enable':
drivers/pci/hotplug/../pci.h:483:47: warning: no return statement in function returning non-void [-Wreturn-type]
483 | static inline int pci_cxl_cache_enable(struct pci_dev *dev) {}
| ^~~~~~~
vim +481 drivers/pci/hotplug/../pci.h
471
472 #ifdef CONFIG_PCI_CXL
473 /* Compute eXpress Link */
474 void pci_cxl_init(struct pci_dev *dev);
475 int pci_cxl_mem_enable(struct pci_dev *dev);
476 void pci_cxl_mem_disable(struct pci_dev *dev);
477 int pci_cxl_cache_enable(struct pci_dev *dev);
478 void pci_cxl_cache_disable(struct pci_dev *dev);
479 #else
480 static inline void pci_cxl_init(struct pci_dev *dev) { }
> 481 static inline int pci_cxl_mem_enable(struct pci_dev *dev) {}
482 static inline void pci_cxl_mem_disable(struct pci_dev *dev) {}
483 static inline int pci_cxl_cache_enable(struct pci_dev *dev) {}
484 static inline void pci_cxl_cache_disable(struct pci_dev *dev) {}
485 #endif
486
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On 18 May 2020, at 9:55, Bjorn Helgaas wrote: > On Mon, May 18, 2020 at 09:35:23AM -0700, Sean V Kelley wrote: >> With these helpers, a device driver can enable/disable access to >> CXL.mem and CXL.cache. Note that the device driver is responsible for >> managing the memory area. >> >> Signed-off-by: Sean V Kelley <sean.v.kelley@linux.intel.com> >> --- >> drivers/pci/cxl.c | 93 >> ++++++++++++++++++++++++++++++++++++++++++++--- >> drivers/pci/pci.h | 8 ++++ >> 2 files changed, 96 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/pci/cxl.c b/drivers/pci/cxl.c >> index 4437ca69ad33..0d0a1b82ea98 100644 >> --- a/drivers/pci/cxl.c >> +++ b/drivers/pci/cxl.c >> @@ -24,6 +24,88 @@ >> #define PCI_CXL_HDM_COUNT(reg) (((reg) & (3 << 4)) >> 4) >> #define PCI_CXL_VIRAL BIT(14) >> >> +#define PCI_CXL_CONFIG_LOCK BIT(0) >> + >> +static void pci_cxl_unlock(struct pci_dev *dev) >> +{ >> + int pos = dev->cxl_cap; >> + u16 lock; >> + >> + pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); >> + lock &= ~PCI_CXL_CONFIG_LOCK; >> + pci_write_config_word(dev, pos + PCI_CXL_LOCK, lock); >> +} >> + >> +static void pci_cxl_lock(struct pci_dev *dev) >> +{ >> + int pos = dev->cxl_cap; >> + u16 lock; >> + >> + pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); >> + lock |= PCI_CXL_CONFIG_LOCK; >> + pci_write_config_word(dev, pos + PCI_CXL_LOCK, lock); >> +} > > Maybe a tiny comment about what PCI_CXL_CONFIG_LOCK does? I guess > these functions don't enforce mutual exclusion (as I first expected a > "lock" function to do); they're simply telling the device to accept > CTRL changes or something? Agree will add. DVSEC Lock is intended to signify whether the CTRL bits may be changed. It locks certain registers and makes them RO. This lock is designed to prevent future changes to configuration. Not to be confused with enforcing mutual exclusion. > >> +static int pci_cxl_enable_disable_feature(struct pci_dev *dev, int >> enable, >> + u16 feature) >> +{ >> + int pos = dev->cxl_cap; >> + int ret; >> + u16 reg; >> + >> + if (!dev->cxl_cap) >> + return -EINVAL; > > "pos" > > "pos" is the typical name for things like this, but sometimes I think > the code would be more descriptive if we used things like "cxl", > "aer", etc. Will do, makes sense. > >> + >> + /* Only for PCIe */ >> + if (!pci_is_pcie(dev)) >> + return -EINVAL; > > Probably not necessary here. We already checked it in pci_cxi_init(). You are correct. Will remove. > >> + /* Only for Device 0 Function 0, Root Complex Integrated Endpoints >> */ >> + if (dev->devfn != 0 || (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END)) >> + return -EINVAL; >> + >> + pci_cxl_unlock(dev); >> + ret = pci_read_config_word(dev, pos + PCI_CXL_CTRL, ®); >> + if (ret) >> + goto lock; >> + >> + if (enable) >> + reg |= feature; >> + else >> + reg &= ~feature; >> + >> + ret = pci_write_config_word(dev, pos + PCI_CXL_CTRL, reg); >> + >> +lock: >> + pci_cxl_lock(dev); >> + >> + return ret; >> +} >> + >> +int pci_cxl_mem_enable(struct pci_dev *dev) >> +{ >> + return pci_cxl_enable_disable_feature(dev, true, PCI_CXL_MEM); >> +} >> +EXPORT_SYMBOL_GPL(pci_cxl_mem_enable); >> + >> +void pci_cxl_mem_disable(struct pci_dev *dev) >> +{ >> + pci_cxl_enable_disable_feature(dev, false, PCI_CXL_MEM); >> +} >> +EXPORT_SYMBOL_GPL(pci_cxl_mem_disable); >> + >> +int pci_cxl_cache_enable(struct pci_dev *dev) >> +{ >> + return pci_cxl_enable_disable_feature(dev, true, PCI_CXL_CACHE); >> +} >> +EXPORT_SYMBOL_GPL(pci_cxl_cache_enable); >> + >> +void pci_cxl_cache_disable(struct pci_dev *dev) >> +{ >> + pci_cxl_enable_disable_feature(dev, false, PCI_CXL_CACHE); >> +} >> +EXPORT_SYMBOL_GPL(pci_cxl_cache_disable); >> + >> /* >> * pci_find_cxl_capability - Identify and return offset to >> Vendor-Specific >> * capabilities. >> @@ -73,11 +155,6 @@ void pci_cxl_init(struct pci_dev *dev) >> dev->cxl_cap = pos; >> >> pci_read_config_word(dev, pos + PCI_CXL_CAP, &cap); >> - pci_read_config_word(dev, pos + PCI_CXL_CTRL, &ctrl); >> - pci_read_config_word(dev, pos + PCI_CXL_STS, &status); >> - pci_read_config_word(dev, pos + PCI_CXL_CTRL2, &ctrl2); >> - pci_read_config_word(dev, pos + PCI_CXL_STS2, &status2); >> - pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); >> >> pci_info(dev, "CXL: Cache%c IO%c Mem%c Viral%c HDMCount %d\n", >> FLAG(cap, PCI_CXL_CACHE), >> @@ -86,6 +163,12 @@ void pci_cxl_init(struct pci_dev *dev) >> FLAG(cap, PCI_CXL_VIRAL), >> PCI_CXL_HDM_COUNT(cap)); >> >> + pci_read_config_word(dev, pos + PCI_CXL_CTRL, &ctrl); >> + pci_read_config_word(dev, pos + PCI_CXL_STS, &status); >> + pci_read_config_word(dev, pos + PCI_CXL_CTRL2, &ctrl2); >> + pci_read_config_word(dev, pos + PCI_CXL_STS2, &status2); >> + pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); > > This looks like it's just moving these reads around and isn't actually > related to this patch. Put them where you want them in the first > place so the move doesn't clutter this patch. Looks that way. I will clean it up. > >> pci_info(dev, "CXL: cap ctrl status ctrl2 status2 lock\n"); >> pci_info(dev, "CXL: %04x %04x %04x %04x %04x %04x\n", >> cap, ctrl, status, ctrl2, status2, lock); >> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h >> index d9905e2dee95..6336e16565ac 100644 >> --- a/drivers/pci/pci.h >> +++ b/drivers/pci/pci.h >> @@ -472,8 +472,16 @@ static inline void pci_restore_ats_state(struct >> pci_dev *dev) { } >> #ifdef CONFIG_PCI_CXL >> /* Compute eXpress Link */ >> void pci_cxl_init(struct pci_dev *dev); >> +int pci_cxl_mem_enable(struct pci_dev *dev); >> +void pci_cxl_mem_disable(struct pci_dev *dev); >> +int pci_cxl_cache_enable(struct pci_dev *dev); >> +void pci_cxl_cache_disable(struct pci_dev *dev); >> #else >> static inline void pci_cxl_init(struct pci_dev *dev) { } >> +static inline int pci_cxl_mem_enable(struct pci_dev *dev) {} >> +static inline void pci_cxl_mem_disable(struct pci_dev *dev) {} >> +static inline int pci_cxl_cache_enable(struct pci_dev *dev) {} >> +static inline void pci_cxl_cache_disable(struct pci_dev *dev) {} > > The stubs in this file aren't completely consistent, but you can be at > least locally consistent by including a space between the "{ }". Will do. Thanks, Sean > >> #endif >> >> #ifdef CONFIG_PCI_PRI >> -- >> 2.26.2 >>
On Mon, May 18, 2020 at 09:35:23AM -0700, Sean V Kelley wrote: > With these helpers, a device driver can enable/disable access to > CXL.mem and CXL.cache. Note that the device driver is responsible for > managing the memory area. Who is going to call these? Please don't submit new APIs without users.
diff --git a/drivers/pci/cxl.c b/drivers/pci/cxl.c index 4437ca69ad33..0d0a1b82ea98 100644 --- a/drivers/pci/cxl.c +++ b/drivers/pci/cxl.c @@ -24,6 +24,88 @@ #define PCI_CXL_HDM_COUNT(reg) (((reg) & (3 << 4)) >> 4) #define PCI_CXL_VIRAL BIT(14) +#define PCI_CXL_CONFIG_LOCK BIT(0) + +static void pci_cxl_unlock(struct pci_dev *dev) +{ + int pos = dev->cxl_cap; + u16 lock; + + pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); + lock &= ~PCI_CXL_CONFIG_LOCK; + pci_write_config_word(dev, pos + PCI_CXL_LOCK, lock); +} + +static void pci_cxl_lock(struct pci_dev *dev) +{ + int pos = dev->cxl_cap; + u16 lock; + + pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); + lock |= PCI_CXL_CONFIG_LOCK; + pci_write_config_word(dev, pos + PCI_CXL_LOCK, lock); +} + +static int pci_cxl_enable_disable_feature(struct pci_dev *dev, int enable, + u16 feature) +{ + int pos = dev->cxl_cap; + int ret; + u16 reg; + + if (!dev->cxl_cap) + return -EINVAL; + + /* Only for PCIe */ + if (!pci_is_pcie(dev)) + return -EINVAL; + + /* Only for Device 0 Function 0, Root Complex Integrated Endpoints */ + if (dev->devfn != 0 || (pci_pcie_type(dev) != PCI_EXP_TYPE_RC_END)) + return -EINVAL; + + pci_cxl_unlock(dev); + ret = pci_read_config_word(dev, pos + PCI_CXL_CTRL, ®); + if (ret) + goto lock; + + if (enable) + reg |= feature; + else + reg &= ~feature; + + ret = pci_write_config_word(dev, pos + PCI_CXL_CTRL, reg); + +lock: + pci_cxl_lock(dev); + + return ret; +} + +int pci_cxl_mem_enable(struct pci_dev *dev) +{ + return pci_cxl_enable_disable_feature(dev, true, PCI_CXL_MEM); +} +EXPORT_SYMBOL_GPL(pci_cxl_mem_enable); + +void pci_cxl_mem_disable(struct pci_dev *dev) +{ + pci_cxl_enable_disable_feature(dev, false, PCI_CXL_MEM); +} +EXPORT_SYMBOL_GPL(pci_cxl_mem_disable); + +int pci_cxl_cache_enable(struct pci_dev *dev) +{ + return pci_cxl_enable_disable_feature(dev, true, PCI_CXL_CACHE); +} +EXPORT_SYMBOL_GPL(pci_cxl_cache_enable); + +void pci_cxl_cache_disable(struct pci_dev *dev) +{ + pci_cxl_enable_disable_feature(dev, false, PCI_CXL_CACHE); +} +EXPORT_SYMBOL_GPL(pci_cxl_cache_disable); + /* * pci_find_cxl_capability - Identify and return offset to Vendor-Specific * capabilities. @@ -73,11 +155,6 @@ void pci_cxl_init(struct pci_dev *dev) dev->cxl_cap = pos; pci_read_config_word(dev, pos + PCI_CXL_CAP, &cap); - pci_read_config_word(dev, pos + PCI_CXL_CTRL, &ctrl); - pci_read_config_word(dev, pos + PCI_CXL_STS, &status); - pci_read_config_word(dev, pos + PCI_CXL_CTRL2, &ctrl2); - pci_read_config_word(dev, pos + PCI_CXL_STS2, &status2); - pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); pci_info(dev, "CXL: Cache%c IO%c Mem%c Viral%c HDMCount %d\n", FLAG(cap, PCI_CXL_CACHE), @@ -86,6 +163,12 @@ void pci_cxl_init(struct pci_dev *dev) FLAG(cap, PCI_CXL_VIRAL), PCI_CXL_HDM_COUNT(cap)); + pci_read_config_word(dev, pos + PCI_CXL_CTRL, &ctrl); + pci_read_config_word(dev, pos + PCI_CXL_STS, &status); + pci_read_config_word(dev, pos + PCI_CXL_CTRL2, &ctrl2); + pci_read_config_word(dev, pos + PCI_CXL_STS2, &status2); + pci_read_config_word(dev, pos + PCI_CXL_LOCK, &lock); + pci_info(dev, "CXL: cap ctrl status ctrl2 status2 lock\n"); pci_info(dev, "CXL: %04x %04x %04x %04x %04x %04x\n", cap, ctrl, status, ctrl2, status2, lock); diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index d9905e2dee95..6336e16565ac 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -472,8 +472,16 @@ static inline void pci_restore_ats_state(struct pci_dev *dev) { } #ifdef CONFIG_PCI_CXL /* Compute eXpress Link */ void pci_cxl_init(struct pci_dev *dev); +int pci_cxl_mem_enable(struct pci_dev *dev); +void pci_cxl_mem_disable(struct pci_dev *dev); +int pci_cxl_cache_enable(struct pci_dev *dev); +void pci_cxl_cache_disable(struct pci_dev *dev); #else static inline void pci_cxl_init(struct pci_dev *dev) { } +static inline int pci_cxl_mem_enable(struct pci_dev *dev) {} +static inline void pci_cxl_mem_disable(struct pci_dev *dev) {} +static inline int pci_cxl_cache_enable(struct pci_dev *dev) {} +static inline void pci_cxl_cache_disable(struct pci_dev *dev) {} #endif #ifdef CONFIG_PCI_PRI
With these helpers, a device driver can enable/disable access to CXL.mem and CXL.cache. Note that the device driver is responsible for managing the memory area. Signed-off-by: Sean V Kelley <sean.v.kelley@linux.intel.com> --- drivers/pci/cxl.c | 93 ++++++++++++++++++++++++++++++++++++++++++++--- drivers/pci/pci.h | 8 ++++ 2 files changed, 96 insertions(+), 5 deletions(-)