| Message ID | 20201005151126.657029-1-dave.jiang@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add shared workqueue support for idxd driver | expand |
On 05-10-20, 08:11, Dave Jiang wrote: > == Background == > A typical DMA device requires the driver to translate application buffers to hardware addresses, > and a kernel-user transition to notify the hardware of new work. Shared Virtual Addressing (SVA) > allows the processor and device to use the same virtual addresses without requiring software to > translate between the address spaces. ENQCMD is a new instruction on Intel Platforms that allows > user applications to directly notify hardware of new work, much like how doorbells are used in > some hardware, but it carries a payload along with it. ENQCMDS is the supervisor version (ring0) > of ENQCMD. > > == ENQCMDS == > Introduce enqcmds(), a helper funciton that copies an input payload to a 64B aligned > destination and confirms whether the payload was accepted by the device or not. > enqcmds() wraps the new ENQCMDS CPU instruction. The ENQCMDS is a ring 0 CPU instruction that > performs similar to the ENQCMD instruction. Descriptor submission must use ENQCMD(S) for shared > workqueues (swq) on an Intel DSA device. > > == Shared WQ support == > Introduce shared workqueue (swq) support for the idxd driver. The current idxd driver contains > dedicated workqueue (dwq) support only. A dwq accepts descriptors from a MOVDIR64B instruction. > MOVDIR64B is a posted instruction on the PCIe bus, it does not wait for any response from the > device. If the wq is full, submitted descriptors are dropped. A swq utilizes the ENQCMDS in > ring 0, which is a non-posted instruction. The zero flag would be set to 1 if the device rejects > the descriptor or if the wq is full. A swq can be shared between multiple users > (kernel or userspace) due to not having to keep track of the wq full condition for submission. > A swq requires PASID and can only run with SVA support. > > == IDXD SVA support == > Add utilization of PASID to support Shared Virtual Addressing (SVA). With PASID support, > the descriptors can be programmed with host virtual address (HVA) rather than IOVA. > The hardware will work with the IOMMU in fulfilling page requests. With SVA support, > a user app using the char device interface can now submit descriptors without having to pin the > virtual memory range it wants to DMA in its own address space. > > The series does not add SVA support for the dmaengine subsystem. That support is coming at a > later time. Applied, thanks
On Wed, Oct 07, 2020 at 12:31:32PM +0530, Vinod Koul wrote:
> Applied, thanks
I'm tired of repeating what you should've done - your branch doesn't
even build. How did you test it?
Also, what happens if Linus merges your branch first, before tip?
Oh boy.
In file included from ./arch/x86/include/asm/tsc.h:9,
from ./arch/x86/include/asm/timex.h:6,
from ./include/linux/timex.h:65,
from ./include/linux/time32.h:13,
from ./include/linux/time.h:73,
from ./include/linux/stat.h:19,
from ./include/linux/module.h:13,
from drivers/dma/idxd/init.c:5:
drivers/dma/idxd/init.c: In function ‘idxd_init_module’:
drivers/dma/idxd/init.c:526:20: error: ‘X86_FEATURE_ENQCMD’ undeclared (first use in this function); did you mean ‘X86_FEATURE_PCID’?
526 | if (!boot_cpu_has(X86_FEATURE_ENQCMD))
| ^~~~~~~~~~~~~~~~~~
./arch/x86/include/asm/cpufeature.h:118:24: note: in definition of macro ‘cpu_has’
118 | (__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
| ^~~
drivers/dma/idxd/init.c:526:7: note: in expansion of macro ‘boot_cpu_has’
526 | if (!boot_cpu_has(X86_FEATURE_ENQCMD))
| ^~~~~~~~~~~~
drivers/dma/idxd/init.c:526:20: note: each undeclared identifier is reported only once for each function it appears in
526 | if (!boot_cpu_has(X86_FEATURE_ENQCMD))
| ^~~~~~~~~~~~~~~~~~
./arch/x86/include/asm/cpufeature.h:118:24: note: in definition of macro ‘cpu_has’
118 | (__builtin_constant_p(bit) && REQUIRED_MASK_BIT_SET(bit) ? 1 : \
| ^~~
drivers/dma/idxd/init.c:526:7: note: in expansion of macro ‘boot_cpu_has’
526 | if (!boot_cpu_has(X86_FEATURE_ENQCMD))
| ^~~~~~~~~~~~
make[3]: *** [scripts/Makefile.build:283: drivers/dma/idxd/init.o] Error 1
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:500: drivers/dma/idxd] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [scripts/Makefile.build:500: drivers/dma] Error 2
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1788: drivers] Error 2
make: *** Waiting for unfinished jobs....
On 07-10-20, 10:48, Borislav Petkov wrote: > On Wed, Oct 07, 2020 at 12:31:32PM +0530, Vinod Koul wrote: > > Applied, thanks > > I'm tired of repeating what you should've done - your branch doesn't > even build. How did you test it? Right my build failed for x86 and I have dropped these now. I would have expected the dependency to be a signed tag to be cross merged when I was asked to merge this.
On Wed, Oct 07, 2020 at 03:23:13PM +0530, Vinod Koul wrote: > Right my build failed for x86 and I have dropped these now. I would have > expected the dependency to be a signed tag to be cross merged when I was > asked to merge this. I can give you a signed tag is you prefer but that's usually not necessary. You can simply merge tip's x86/pasid branch, then apply those ontop and test. HTH.
On 07-10-20, 12:04, Borislav Petkov wrote: > On Wed, Oct 07, 2020 at 03:23:13PM +0530, Vinod Koul wrote: > > Right my build failed for x86 and I have dropped these now. I would have > > expected the dependency to be a signed tag to be cross merged when I was > > asked to merge this. > > I can give you a signed tag is you prefer but that's usually not That would be better, signed tags are preferred > necessary. You can simply merge tip's x86/pasid branch, then apply those > ontop and test. While at it, it would be good if x86 patches of this series come from your tree, that makes more sense if we are doing a cross merge Thanks
On Wed, Oct 07, 2020 at 08:27:33PM +0530, Vinod Koul wrote: > That would be better, signed tags are preferred ... > While at it, it would be good if x86 patches of this series come from > your tree, that makes more sense if we are doing a cross merge All done, here it is: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/tag/?h=x86_pasid_for_5.10 That's going to be the tag I send to Linus next week too. I'll send it on Monday when the merge window opens so that he can merge it before your branch. HTH.
On 10/7/2020 12:01 AM, Vinod Koul wrote: > On 05-10-20, 08:11, Dave Jiang wrote: > >> == Background == >> A typical DMA device requires the driver to translate application buffers to hardware addresses, >> and a kernel-user transition to notify the hardware of new work. Shared Virtual Addressing (SVA) >> allows the processor and device to use the same virtual addresses without requiring software to >> translate between the address spaces. ENQCMD is a new instruction on Intel Platforms that allows >> user applications to directly notify hardware of new work, much like how doorbells are used in >> some hardware, but it carries a payload along with it. ENQCMDS is the supervisor version (ring0) >> of ENQCMD. >> >> == ENQCMDS == >> Introduce enqcmds(), a helper funciton that copies an input payload to a 64B aligned >> destination and confirms whether the payload was accepted by the device or not. >> enqcmds() wraps the new ENQCMDS CPU instruction. The ENQCMDS is a ring 0 CPU instruction that >> performs similar to the ENQCMD instruction. Descriptor submission must use ENQCMD(S) for shared >> workqueues (swq) on an Intel DSA device. >> >> == Shared WQ support == >> Introduce shared workqueue (swq) support for the idxd driver. The current idxd driver contains >> dedicated workqueue (dwq) support only. A dwq accepts descriptors from a MOVDIR64B instruction. >> MOVDIR64B is a posted instruction on the PCIe bus, it does not wait for any response from the >> device. If the wq is full, submitted descriptors are dropped. A swq utilizes the ENQCMDS in >> ring 0, which is a non-posted instruction. The zero flag would be set to 1 if the device rejects >> the descriptor or if the wq is full. A swq can be shared between multiple users >> (kernel or userspace) due to not having to keep track of the wq full condition for submission. >> A swq requires PASID and can only run with SVA support. >> >> == IDXD SVA support == >> Add utilization of PASID to support Shared Virtual Addressing (SVA). With PASID support, >> the descriptors can be programmed with host virtual address (HVA) rather than IOVA. >> The hardware will work with the IOMMU in fulfilling page requests. With SVA support, >> a user app using the char device interface can now submit descriptors without having to pin the >> virtual memory range it wants to DMA in its own address space. >> >> The series does not add SVA support for the dmaengine subsystem. That support is coming at a >> later time. > > Applied, thanks > Thanks Vinod!