* Re: [PATCH 2/6] device/pci: add cmdmem cap to pci_dev
[not found] <d798a7eb-ceb0-d81e-5422-f9e41058a098@intel.com>
@ 2020-03-31 22:00 ` Bjorn Helgaas
2020-03-31 22:34 ` Thomas Gleixner
2020-04-01 16:37 ` Dave Jiang
0 siblings, 2 replies; 10+ messages in thread
From: Bjorn Helgaas @ 2020-03-31 22:00 UTC (permalink / raw)
To: Dave Jiang
Cc: vkoul, tglx, mingo, bp, hpa, gregkh, arnd, linux-kernel, x86,
dmaengine, dan.j.williams, ashok.raj, fenghua.yu, linux-pci,
tony.luck, jing.lin, sanjay.k.kumar
On Tue, Mar 31, 2020 at 10:59:44AM -0700, Dave Jiang wrote:
> On 3/31/2020 9:03 AM, Bjorn Helgaas wrote:
> > On Mon, Mar 30, 2020 at 02:27:00PM -0700, Dave Jiang wrote:
> > > Since the current accelerator devices do not have standard PCIe capability
> > > enumeration for accepting ENQCMDS yet, for now an attribute of pdev->cmdmem has
> > > been added to struct pci_dev. Currently a PCI quirk must be used for the
> > > devices that have such cap until the PCI cap is standardized. Add a helper
> > > function to provide the check if a device supports the cmdmem capability.
> > >
> > > Such capability is expected to be added to PCIe device cap enumeration in
> > > the future.
> > This needs some sort of thumbnail description of what "synchronous
> > write notification" and "cmdmem" mean.
>
> I will add more explanation.
>
> > Do you have a pointer to a PCI-SIG ECR or similar?
>
> Deferrable Memory Write (DMWr) ECR
>
> https://members.pcisig.com/wg/PCI-SIG/document/13747
>
> From what I'm told it should be available for public review by EOW.
Please use terminology from the spec instead of things like
"synchronous write notification".
AIUI, ENQCMDS is an x86 instruction. That would have no meaning in
the PCIe domain.
I'm not committing to acking any part of this before the ECR is
accepted, but if you're adding support for the feature described by
the ECR, you might as well add support for discovering the DMWr
capability via Device Capabilities 2 as described in the ECR.
If you have hardware that supports DMWr but doesn't advertise it via
Device Capabilities 2, you can always have a quirk that works around
that lack.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/6] device/pci: add cmdmem cap to pci_dev
2020-03-31 22:00 ` [PATCH 2/6] device/pci: add cmdmem cap to pci_dev Bjorn Helgaas
@ 2020-03-31 22:34 ` Thomas Gleixner
2020-04-01 16:37 ` Dave Jiang
1 sibling, 0 replies; 10+ messages in thread
From: Thomas Gleixner @ 2020-03-31 22:34 UTC (permalink / raw)
To: Bjorn Helgaas, Dave Jiang
Cc: vkoul, mingo, bp, hpa, gregkh, arnd, linux-kernel, x86,
dmaengine, dan.j.williams, ashok.raj, fenghua.yu, linux-pci,
tony.luck, jing.lin, sanjay.k.kumar
Bjorn Helgaas <helgaas@kernel.org> writes:
> On Tue, Mar 31, 2020 at 10:59:44AM -0700, Dave Jiang wrote:
>> On 3/31/2020 9:03 AM, Bjorn Helgaas wrote:
>> > On Mon, Mar 30, 2020 at 02:27:00PM -0700, Dave Jiang wrote:
>> > > Since the current accelerator devices do not have standard PCIe capability
>> > > enumeration for accepting ENQCMDS yet, for now an attribute of pdev->cmdmem has
>> > > been added to struct pci_dev. Currently a PCI quirk must be used for the
>> > > devices that have such cap until the PCI cap is standardized. Add a helper
>> > > function to provide the check if a device supports the cmdmem capability.
>> > >
>> > > Such capability is expected to be added to PCIe device cap enumeration in
>> > > the future.
>> > This needs some sort of thumbnail description of what "synchronous
>> > write notification" and "cmdmem" mean.
>>
>> I will add more explanation.
>>
>> > Do you have a pointer to a PCI-SIG ECR or similar?
>>
>> Deferrable Memory Write (DMWr) ECR
>>
>> https://members.pcisig.com/wg/PCI-SIG/document/13747
>>
>> From what I'm told it should be available for public review by EOW.
>
> Please use terminology from the spec instead of things like
> "synchronous write notification".
>
> AIUI, ENQCMDS is an x86 instruction. That would have no meaning in
> the PCIe domain.
>
> I'm not committing to acking any part of this before the ECR is
> accepted, but if you're adding support for the feature described by
> the ECR, you might as well add support for discovering the DMWr
> capability via Device Capabilities 2 as described in the ECR.
Don't worry. There is nothing to decide and ack before the basic
architecture support for ENQCMD[S] is discussed and accepted. The
patches providing this support have been posted 2 hrs before this pile
hit the mailing lists yesterday.
Thanks,
tglx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/6] device/pci: add cmdmem cap to pci_dev
2020-03-31 22:00 ` [PATCH 2/6] device/pci: add cmdmem cap to pci_dev Bjorn Helgaas
2020-03-31 22:34 ` Thomas Gleixner
@ 2020-04-01 16:37 ` Dave Jiang
1 sibling, 0 replies; 10+ messages in thread
From: Dave Jiang @ 2020-04-01 16:37 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: vkoul, tglx, mingo, bp, hpa, gregkh, arnd, linux-kernel, x86,
dmaengine, dan.j.williams, ashok.raj, fenghua.yu, linux-pci,
tony.luck, jing.lin, sanjay.k.kumar
On 3/31/2020 3:00 PM, Bjorn Helgaas wrote:
> On Tue, Mar 31, 2020 at 10:59:44AM -0700, Dave Jiang wrote:
>> On 3/31/2020 9:03 AM, Bjorn Helgaas wrote:
>>> On Mon, Mar 30, 2020 at 02:27:00PM -0700, Dave Jiang wrote:
>>>> Since the current accelerator devices do not have standard PCIe capability
>>>> enumeration for accepting ENQCMDS yet, for now an attribute of pdev->cmdmem has
>>>> been added to struct pci_dev. Currently a PCI quirk must be used for the
>>>> devices that have such cap until the PCI cap is standardized. Add a helper
>>>> function to provide the check if a device supports the cmdmem capability.
>>>>
>>>> Such capability is expected to be added to PCIe device cap enumeration in
>>>> the future.
>>> This needs some sort of thumbnail description of what "synchronous
>>> write notification" and "cmdmem" mean.
>>
>> I will add more explanation.
>>
>>> Do you have a pointer to a PCI-SIG ECR or similar?
>>
>> Deferrable Memory Write (DMWr) ECR
>>
>> https://members.pcisig.com/wg/PCI-SIG/document/13747
>>
>> From what I'm told it should be available for public review by EOW.
>
> Please use terminology from the spec instead of things like
> "synchronous write notification".
>
> AIUI, ENQCMDS is an x86 instruction. That would have no meaning in
> the PCIe domain.
>
> I'm not committing to acking any part of this before the ECR is
> accepted, but if you're adding support for the feature described by
> the ECR, you might as well add support for discovering the DMWr
> capability via Device Capabilities 2 as described in the ECR.
I'll look into adding the support for the ECR.
>
> If you have hardware that supports DMWr but doesn't advertise it via
> Device Capabilities 2, you can always have a quirk that works around
> that lack.
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 0/6] Add shared workqueue support for idxd driver
@ 2020-03-30 21:26 Dave Jiang
2020-03-30 21:27 ` [PATCH 2/6] device/pci: add cmdmem cap to pci_dev Dave Jiang
0 siblings, 1 reply; 10+ messages in thread
From: Dave Jiang @ 2020-03-30 21:26 UTC (permalink / raw)
To: vkoul, tglx, mingo, bp, hpa, bhelgaas, gregkh, arnd
Cc: linux-kernel, x86, dmaengine, dan.j.williams, ashok.raj,
fenghua.yu, linux-pci, tony.luck, jing.lin, sanjay.k.kumar
The patch series breaks down into following parts:
Patch 1: x86 arch, add a new I/O accessor based on ENQCMDS
Patches 2,3: PCI
Patch 4: device
Patches 5,6: idxd driver shared WQ support
Driver stage 1 postings for context: [1]
This patch series is dependent on Fenghua's "Tag application address space
for devices" patch series for the ENQCMD CPU command enumeration and the
PASID MSR support. [2]
This patch series introduces support for shared workqueue (swq) for the
idxd driver that is supported by the Intel Data Streaming Accelerator (DSA). In
the stage 1 patch series posting, only dedicated workqueues (dwq) are
supported. Another major feature being introduced is Shared Virtual Memory (SVM)
support.
In this patch series, we introduce the iosubmit_cmds512_sync() command.
This function enables the calling of the new ENQCMDS CPU instruction on the
Intel CPU. ENQCMDS is a ring0 CPU instruction that performs similar function as
the ENQCMD. The CPUID capability bit will enumerate ENQCMD is supported,
which implies that ENQCMDS is supported as well. The instruction creates a
non-posted write command on the PCIe bus and atomically writes all 64 bytes of a
command descriptor to the accelerator device's special BAR address. The device can
reject the command by using the zero flag. See Intel SDM [2] for additional
details. The ENQCMDS instruction is used for submitting command descriptors
to the swq. With this instruction, multiple "users" in the kernel can submit
to the swq with the synchronization done by the hardware. When the swq is
full, a 1 will be returned indicate the wq being busy. This is different than a
dwq where the command will be silently dropped without response. The dwq
requires the software to track the queue depth of a dwq.
The attribute of cmdmem device capability is being introduced to the PCI
device with ongoing discussion of adding the ability to enumerate PCIe capability
that provides the support of such device MMIO regions. A
device_supports_cmdmem(struct device *) helper function is introduced to
initially support devices that has this capability. Since the standardized
way to detect such capability is not available yet, a PCI quirk is defined
for the DSA device.
ENQCMDS moves a command descriptor (64 bytes) from memory to specific MMIO
regions on a device that supports this capability. Like MMIO, the "handle"
for these queues is just a pointer to the address of the queue in MMIO memory.
These specific MMIO regions on the device are called portals. To support
these portals, wrappers functions are introduced for ioremap() to make it clear
that the special MMIO regions that expects a response from a non-posted PCI
write are being ioremaped.
The support of SVM is done through Process Address Space ID (PASID). With
SVM support, the DMA descriptors can be programmed with virtual address for
source and destination addresses. When a page fault is encountered, the device can
fault in the memory page needed to complete the operation through the
IOMMU. This makes calling the dma_(un)map_* API calls unnecessary which reduces
some software latencies. Both swq and dwq can support SVM.
Swq enabling has been added to the idxd char device driver to allow
exposure of the swq to the user space. User apps can call ENQCMD to submit
descriptors directly to a swq after calling mmap() on the portal. ENQCMD is
similar to the ENQCMDS instruction, but is only available to ring3
(application) code. The primary difference is that ENQCMD obtains the PASID for the
request from the IA32_PASID MSR to ensure that all virtual addresses specified by
the user are interpreted in the address space of the process that executed
ENQCMD. With the SVM enabling, the user no longer has to pin the memory and program
IO Virtual Address (IOVA) as source and address. Virtual addresses can be
programmed in the command descriptor and be submitted to the device. The
SVM handling is done through the usage of PASID. For more detailed explanation
on how ENQCMD and PASID interaction works please refer to Fenghua's submission
cover letter [6]. Multiple user apps can easily share a swq with the ENQCMD
instruction without needing software sychronization.
[1]: https://lore.kernel.org/lkml/157965011794.73301.15960052071729101309.stgit@djiang5-desk3.ch.intel.com/
[2]: https://lore.kernel.org/lkml/1585596788-193989-1-git-send-email-fenghua.yu@intel.com/
[3]: https://software.intel.com/en-us/articles/intel-sdm
[4]: https://software.intel.com/en-us/download/intel-scalable-io-virtualization-technical-specification
[5]: https://software.intel.com/en-us/download/intel-data-streaming-accelerator-preliminary-architecture-specification
[6]: https://01.org/blogs/2019/introducing-intel-data-streaming-accelerator
[7]: https://intel.github.io/idxd/
[8]: https://github.com/intel/idxd-driver idxd-stage2
---
Dave Jiang (6):
x86/asm: add iosubmit_cmds512_sync() based on enqcmds
device/pci: add cmdmem cap to pci_dev
pci: add PCI quirk cmdmem fixup for Intel DSA device
device: add cmdmem support for MMIO address
dmaengine: idxd: add shared workqueue support
dmaengine: idxd: add ABI documentation for shared wq
Documentation/ABI/stable/sysfs-driver-dma-idxd | 14 ++
arch/x86/include/asm/io.h | 37 ++++++
drivers/base/core.c | 13 ++
drivers/dma/Kconfig | 4 +
drivers/dma/idxd/cdev.c | 46 +++++++-
drivers/dma/idxd/device.c | 122 ++++++++++++++++++--
drivers/dma/idxd/dma.c | 2
drivers/dma/idxd/idxd.h | 13 ++
drivers/dma/idxd/init.c | 92 ++++++++++++---
drivers/dma/idxd/irq.c | 147 ++++++++++++++++++++++--
drivers/dma/idxd/submit.c | 119 ++++++++++++++-----
drivers/dma/idxd/sysfs.c | 133 ++++++++++++++++++++++
drivers/pci/quirks.c | 11 ++
include/linux/device.h | 2
include/linux/io.h | 4 +
include/linux/pci.h | 1
lib/devres.c | 36 ++++++
17 files changed, 721 insertions(+), 75 deletions(-)
--
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/6] device/pci: add cmdmem cap to pci_dev
2020-03-30 21:26 [PATCH 0/6] Add shared workqueue support for idxd driver Dave Jiang
@ 2020-03-30 21:27 ` Dave Jiang
2020-03-31 10:04 ` Greg KH
2020-03-31 16:03 ` Bjorn Helgaas
0 siblings, 2 replies; 10+ messages in thread
From: Dave Jiang @ 2020-03-30 21:27 UTC (permalink / raw)
To: vkoul, tglx, mingo, bp, hpa, bhelgaas, gregkh, arnd
Cc: linux-kernel, x86, dmaengine, dan.j.williams, ashok.raj,
fenghua.yu, linux-pci, tony.luck, jing.lin, sanjay.k.kumar
Since the current accelerator devices do not have standard PCIe capability
enumeration for accepting ENQCMDS yet, for now an attribute of pdev->cmdmem has
been added to struct pci_dev. Currently a PCI quirk must be used for the
devices that have such cap until the PCI cap is standardized. Add a helper
function to provide the check if a device supports the cmdmem capability.
Such capability is expected to be added to PCIe device cap enumeration in
the future.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
drivers/base/core.c | 13 +++++++++++++
include/linux/device.h | 2 ++
include/linux/pci.h | 1 +
3 files changed, 16 insertions(+)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index dbb0f9130f42..cd9f5b040ed4 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -27,6 +27,7 @@
#include <linux/netdevice.h>
#include <linux/sched/signal.h>
#include <linux/sysfs.h>
+#include <linux/pci.h>
#include "base.h"
#include "power/power.h"
@@ -3790,3 +3791,15 @@ int device_match_any(struct device *dev, const void *unused)
return 1;
}
EXPORT_SYMBOL_GPL(device_match_any);
+
+bool device_supports_cmdmem(struct device *dev)
+{
+ struct pci_dev *pdev;
+
+ if (!dev_is_pci(dev))
+ return false;
+
+ pdev = to_pci_dev(dev);
+ return pdev->cmdmem;
+}
+EXPORT_SYMBOL_GPL(device_supports_cmdmem);
diff --git a/include/linux/device.h b/include/linux/device.h
index fa04dfd22bbc..3e787d3de435 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -809,6 +809,8 @@ static inline bool dev_has_sync_state(struct device *dev)
return false;
}
+extern bool device_supports_cmdmem(struct device *dev);
+
/*
* High level routines for use by the bus drivers
*/
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 3840a541a9de..0bc5d581f20e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -422,6 +422,7 @@ struct pci_dev {
unsigned int is_probed:1; /* Device probing in progress */
unsigned int link_active_reporting:1;/* Device capable of reporting link active */
unsigned int no_vf_scan:1; /* Don't scan for VFs after IOV enablement */
+ unsigned int cmdmem:1; /* MMIO writes support command mem region with synchronous write notification */
pci_dev_flags_t dev_flags;
atomic_t enable_cnt; /* pci_enable_device has been called */
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/6] device/pci: add cmdmem cap to pci_dev
2020-03-30 21:27 ` [PATCH 2/6] device/pci: add cmdmem cap to pci_dev Dave Jiang
@ 2020-03-31 10:04 ` Greg KH
2020-03-31 17:07 ` Dave Jiang
2020-03-31 16:03 ` Bjorn Helgaas
1 sibling, 1 reply; 10+ messages in thread
From: Greg KH @ 2020-03-31 10:04 UTC (permalink / raw)
To: Dave Jiang
Cc: vkoul, tglx, mingo, bp, hpa, bhelgaas, arnd, linux-kernel, x86,
dmaengine, dan.j.williams, ashok.raj, fenghua.yu, linux-pci,
tony.luck, jing.lin, sanjay.k.kumar
On Mon, Mar 30, 2020 at 02:27:00PM -0700, Dave Jiang wrote:
> Since the current accelerator devices do not have standard PCIe capability
> enumeration for accepting ENQCMDS yet, for now an attribute of pdev->cmdmem has
> been added to struct pci_dev. Currently a PCI quirk must be used for the
> devices that have such cap until the PCI cap is standardized. Add a helper
> function to provide the check if a device supports the cmdmem capability.
>
> Such capability is expected to be added to PCIe device cap enumeration in
> the future.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/base/core.c | 13 +++++++++++++
> include/linux/device.h | 2 ++
> include/linux/pci.h | 1 +
> 3 files changed, 16 insertions(+)
>
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index dbb0f9130f42..cd9f5b040ed4 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -27,6 +27,7 @@
> #include <linux/netdevice.h>
> #include <linux/sched/signal.h>
> #include <linux/sysfs.h>
> +#include <linux/pci.h>
>
> #include "base.h"
> #include "power/power.h"
> @@ -3790,3 +3791,15 @@ int device_match_any(struct device *dev, const void *unused)
> return 1;
> }
> EXPORT_SYMBOL_GPL(device_match_any);
> +
> +bool device_supports_cmdmem(struct device *dev)
> +{
> + struct pci_dev *pdev;
> +
> + if (!dev_is_pci(dev))
> + return false;
> +
> + pdev = to_pci_dev(dev);
> + return pdev->cmdmem;
> +}
> +EXPORT_SYMBOL_GPL(device_supports_cmdmem);
Why would a pci-specific function like this be ok to have in the driver
core? Please keep it in the pci core code instead.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/6] device/pci: add cmdmem cap to pci_dev
2020-03-31 10:04 ` Greg KH
@ 2020-03-31 17:07 ` Dave Jiang
2020-03-31 17:24 ` Greg KH
0 siblings, 1 reply; 10+ messages in thread
From: Dave Jiang @ 2020-03-31 17:07 UTC (permalink / raw)
To: Greg KH
Cc: vkoul, tglx, mingo, bp, hpa, bhelgaas, arnd, linux-kernel, x86,
dmaengine, dan.j.williams, ashok.raj, fenghua.yu, linux-pci,
tony.luck, jing.lin, sanjay.k.kumar
On 3/31/2020 3:04 AM, Greg KH wrote:
> On Mon, Mar 30, 2020 at 02:27:00PM -0700, Dave Jiang wrote:
>> Since the current accelerator devices do not have standard PCIe capability
>> enumeration for accepting ENQCMDS yet, for now an attribute of pdev->cmdmem has
>> been added to struct pci_dev. Currently a PCI quirk must be used for the
>> devices that have such cap until the PCI cap is standardized. Add a helper
>> function to provide the check if a device supports the cmdmem capability.
>>
>> Such capability is expected to be added to PCIe device cap enumeration in
>> the future.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> drivers/base/core.c | 13 +++++++++++++
>> include/linux/device.h | 2 ++
>> include/linux/pci.h | 1 +
>> 3 files changed, 16 insertions(+)
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index dbb0f9130f42..cd9f5b040ed4 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -27,6 +27,7 @@
>> #include <linux/netdevice.h>
>> #include <linux/sched/signal.h>
>> #include <linux/sysfs.h>
>> +#include <linux/pci.h>
>>
>> #include "base.h"
>> #include "power/power.h"
>> @@ -3790,3 +3791,15 @@ int device_match_any(struct device *dev, const void *unused)
>> return 1;
>> }
>> EXPORT_SYMBOL_GPL(device_match_any);
>> +
>> +bool device_supports_cmdmem(struct device *dev)
>> +{
>> + struct pci_dev *pdev;
>> +
>> + if (!dev_is_pci(dev))
>> + return false;
>> +
>> + pdev = to_pci_dev(dev);
>> + return pdev->cmdmem;
>> +}
>> +EXPORT_SYMBOL_GPL(device_supports_cmdmem);
> Why would a pci-specific function like this be ok to have in the driver
> core? Please keep it in the pci core code instead.
The original thought was to introduce a new arch level memory mapping
semantic. If you feel this should be PCI exclusive, should we make the
ioremap routines for this memory type pci specific as well?
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/6] device/pci: add cmdmem cap to pci_dev
2020-03-31 17:07 ` Dave Jiang
@ 2020-03-31 17:24 ` Greg KH
2020-03-31 17:38 ` Dave Jiang
0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2020-03-31 17:24 UTC (permalink / raw)
To: Dave Jiang
Cc: vkoul, tglx, mingo, bp, hpa, bhelgaas, arnd, linux-kernel, x86,
dmaengine, dan.j.williams, ashok.raj, fenghua.yu, linux-pci,
tony.luck, jing.lin, sanjay.k.kumar
On Tue, Mar 31, 2020 at 10:07:07AM -0700, Dave Jiang wrote:
>
> On 3/31/2020 3:04 AM, Greg KH wrote:
> > On Mon, Mar 30, 2020 at 02:27:00PM -0700, Dave Jiang wrote:
> > > Since the current accelerator devices do not have standard PCIe capability
> > > enumeration for accepting ENQCMDS yet, for now an attribute of pdev->cmdmem has
> > > been added to struct pci_dev. Currently a PCI quirk must be used for the
> > > devices that have such cap until the PCI cap is standardized. Add a helper
> > > function to provide the check if a device supports the cmdmem capability.
> > >
> > > Such capability is expected to be added to PCIe device cap enumeration in
> > > the future.
> > >
> > > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> > > ---
> > > drivers/base/core.c | 13 +++++++++++++
> > > include/linux/device.h | 2 ++
> > > include/linux/pci.h | 1 +
> > > 3 files changed, 16 insertions(+)
> > >
> > > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > > index dbb0f9130f42..cd9f5b040ed4 100644
> > > --- a/drivers/base/core.c
> > > +++ b/drivers/base/core.c
> > > @@ -27,6 +27,7 @@
> > > #include <linux/netdevice.h>
> > > #include <linux/sched/signal.h>
> > > #include <linux/sysfs.h>
> > > +#include <linux/pci.h>
> > > #include "base.h"
> > > #include "power/power.h"
> > > @@ -3790,3 +3791,15 @@ int device_match_any(struct device *dev, const void *unused)
> > > return 1;
> > > }
> > > EXPORT_SYMBOL_GPL(device_match_any);
> > > +
> > > +bool device_supports_cmdmem(struct device *dev)
> > > +{
> > > + struct pci_dev *pdev;
> > > +
> > > + if (!dev_is_pci(dev))
> > > + return false;
> > > +
> > > + pdev = to_pci_dev(dev);
> > > + return pdev->cmdmem;
> > > +}
> > > +EXPORT_SYMBOL_GPL(device_supports_cmdmem);
> > Why would a pci-specific function like this be ok to have in the driver
> > core? Please keep it in the pci core code instead.
>
> The original thought was to introduce a new arch level memory mapping
> semantic.
Please do not. Also, that's not what you are doing here from what I can
tell.
> If you feel this should be PCI exclusive, should we make the ioremap
> routines for this memory type pci specific as well?
Why wouldn't it be? Is this needed anywhere else?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/6] device/pci: add cmdmem cap to pci_dev
2020-03-31 17:24 ` Greg KH
@ 2020-03-31 17:38 ` Dave Jiang
0 siblings, 0 replies; 10+ messages in thread
From: Dave Jiang @ 2020-03-31 17:38 UTC (permalink / raw)
To: Greg KH
Cc: vkoul, tglx, mingo, bp, hpa, bhelgaas, arnd, linux-kernel, x86,
dmaengine, dan.j.williams, ashok.raj, fenghua.yu, linux-pci,
tony.luck, jing.lin, sanjay.k.kumar
On 3/31/2020 10:24 AM, Greg KH wrote:
> On Tue, Mar 31, 2020 at 10:07:07AM -0700, Dave Jiang wrote:
>> On 3/31/2020 3:04 AM, Greg KH wrote:
>>> On Mon, Mar 30, 2020 at 02:27:00PM -0700, Dave Jiang wrote:
>>>> Since the current accelerator devices do not have standard PCIe capability
>>>> enumeration for accepting ENQCMDS yet, for now an attribute of pdev->cmdmem has
>>>> been added to struct pci_dev. Currently a PCI quirk must be used for the
>>>> devices that have such cap until the PCI cap is standardized. Add a helper
>>>> function to provide the check if a device supports the cmdmem capability.
>>>>
>>>> Such capability is expected to be added to PCIe device cap enumeration in
>>>> the future.
>>>>
>>>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>>> ---
>>>> drivers/base/core.c | 13 +++++++++++++
>>>> include/linux/device.h | 2 ++
>>>> include/linux/pci.h | 1 +
>>>> 3 files changed, 16 insertions(+)
>>>>
>>>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>>>> index dbb0f9130f42..cd9f5b040ed4 100644
>>>> --- a/drivers/base/core.c
>>>> +++ b/drivers/base/core.c
>>>> @@ -27,6 +27,7 @@
>>>> #include <linux/netdevice.h>
>>>> #include <linux/sched/signal.h>
>>>> #include <linux/sysfs.h>
>>>> +#include <linux/pci.h>
>>>> #include "base.h"
>>>> #include "power/power.h"
>>>> @@ -3790,3 +3791,15 @@ int device_match_any(struct device *dev, const void *unused)
>>>> return 1;
>>>> }
>>>> EXPORT_SYMBOL_GPL(device_match_any);
>>>> +
>>>> +bool device_supports_cmdmem(struct device *dev)
>>>> +{
>>>> + struct pci_dev *pdev;
>>>> +
>>>> + if (!dev_is_pci(dev))
>>>> + return false;
>>>> +
>>>> + pdev = to_pci_dev(dev);
>>>> + return pdev->cmdmem;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(device_supports_cmdmem);
>>> Why would a pci-specific function like this be ok to have in the driver
>>> core? Please keep it in the pci core code instead.
>> The original thought was to introduce a new arch level memory mapping
>> semantic.
> Please do not. Also, that's not what you are doing here from what I can
> tell.
>
>> If you feel this should be PCI exclusive, should we make the ioremap
>> routines for this memory type pci specific as well?
> Why wouldn't it be? Is this needed anywhere else?
Ok I'll make this pci specific.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/6] device/pci: add cmdmem cap to pci_dev
2020-03-30 21:27 ` [PATCH 2/6] device/pci: add cmdmem cap to pci_dev Dave Jiang
2020-03-31 10:04 ` Greg KH
@ 2020-03-31 16:03 ` Bjorn Helgaas
2020-03-31 21:44 ` Dave Jiang
1 sibling, 1 reply; 10+ messages in thread
From: Bjorn Helgaas @ 2020-03-31 16:03 UTC (permalink / raw)
To: Dave Jiang
Cc: vkoul, tglx, mingo, bp, hpa, gregkh, arnd, linux-kernel, x86,
dmaengine, dan.j.williams, ashok.raj, fenghua.yu, linux-pci,
tony.luck, jing.lin, sanjay.k.kumar
On Mon, Mar 30, 2020 at 02:27:00PM -0700, Dave Jiang wrote:
> Since the current accelerator devices do not have standard PCIe capability
> enumeration for accepting ENQCMDS yet, for now an attribute of pdev->cmdmem has
> been added to struct pci_dev. Currently a PCI quirk must be used for the
> devices that have such cap until the PCI cap is standardized. Add a helper
> function to provide the check if a device supports the cmdmem capability.
>
> Such capability is expected to be added to PCIe device cap enumeration in
> the future.
This needs some sort of thumbnail description of what "synchronous
write notification" and "cmdmem" mean.
Do you have a pointer to a PCI-SIG ECR or similar?
Your window size seems to be 85 or so. It would be easier if you used
80 and wrapped the commit log to fit in 75 columns so it looks decent
when "git log" indents it by 4.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/6] device/pci: add cmdmem cap to pci_dev
2020-03-31 16:03 ` Bjorn Helgaas
@ 2020-03-31 21:44 ` Dave Jiang
0 siblings, 0 replies; 10+ messages in thread
From: Dave Jiang @ 2020-03-31 21:44 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: vkoul, tglx, mingo, bp, hpa, gregkh, arnd, linux-kernel, x86,
dmaengine, dan.j.williams, ashok.raj, fenghua.yu, linux-pci,
tony.luck, jing.lin, sanjay.k.kumar
On 3/31/2020 9:03 AM, Bjorn Helgaas wrote:
> On Mon, Mar 30, 2020 at 02:27:00PM -0700, Dave Jiang wrote:
>> Since the current accelerator devices do not have standard PCIe capability
>> enumeration for accepting ENQCMDS yet, for now an attribute of pdev->cmdmem has
>> been added to struct pci_dev. Currently a PCI quirk must be used for the
>> devices that have such cap until the PCI cap is standardized. Add a helper
>> function to provide the check if a device supports the cmdmem capability.
>>
>> Such capability is expected to be added to PCIe device cap enumeration in
>> the future.
Re-send. My misconfigured mail client caused mailing lists to bounce the
send.
>
> This needs some sort of thumbnail description of what "synchronous
> write notification" and "cmdmem" mean.
I will add more explanation.
>
> Do you have a pointer to a PCI-SIG ECR or similar?
Deferrable Memory Write (DMWr) ECR
https://members.pcisig.com/wg/PCI-SIG/document/13747
From what I'm told it should be available for public review by EOW.
>
> Your window size seems to be 85 or so. It would be easier if you used
> 80 and wrapped the commit log to fit in 75 columns so it looks decent
> when "git log" indents it by 4.
>
Ok I will fix.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-04-01 16:38 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <d798a7eb-ceb0-d81e-5422-f9e41058a098@intel.com>
2020-03-31 22:00 ` [PATCH 2/6] device/pci: add cmdmem cap to pci_dev Bjorn Helgaas
2020-03-31 22:34 ` Thomas Gleixner
2020-04-01 16:37 ` Dave Jiang
2020-03-30 21:26 [PATCH 0/6] Add shared workqueue support for idxd driver Dave Jiang
2020-03-30 21:27 ` [PATCH 2/6] device/pci: add cmdmem cap to pci_dev Dave Jiang
2020-03-31 10:04 ` Greg KH
2020-03-31 17:07 ` Dave Jiang
2020-03-31 17:24 ` Greg KH
2020-03-31 17:38 ` Dave Jiang
2020-03-31 16:03 ` Bjorn Helgaas
2020-03-31 21:44 ` Dave Jiang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).