Re: [PATCH 6/8] VMCI: dma dg: allocate send and receive buffers for DMA datagrams
Hi Jorgen,
I love your patch! Perhaps something to improve:
[auto build test WARNING on char-misc/char-misc-testing]
[also build test WARNING on linux/master linus/master v5.17-rc2 next-20220202]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Jorgen-Hansen/VMCI-dma-dg-Add-support-for-DMA-datagrams/20220202-230034
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
7ab004dbcbee38b8a70798835d3ffcd97a985a5e
config: i386-randconfig-s002
(https://download.01.org/0day-ci/archive/20220203/202202030713.sj9t0zv6-...@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-dirty
#
https://github.com/0day-ci/linux/commit/a5ca330527cc5a2ed5eba621707076ab15d856e5
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Jorgen-Hansen/VMCI-dma-dg-Add-support-for-DMA-datagrams/20220202-230034
git checkout a5ca330527cc5a2ed5eba621707076ab15d856e5
# save the config file to linux build tree
mkdir build_dir
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir
ARCH=i386 SHELL=/bin/bash drivers/misc/vmw_vmci/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
sparse warnings: (new ones prefixed by >>)
drivers/misc/vmw_vmci/vmci_guest.c:105:45: sparse: sparse: incorrect type in
argument 1 (different address spaces) @@ expected void const volatile
[noderef] __iomem *addr @@ got char * @@
drivers/misc/vmw_vmci/vmci_guest.c:105:45: sparse: expected void const
volatile [noderef] __iomem *addr
drivers/misc/vmw_vmci/vmci_guest.c:105:45: sparse: got char *
drivers/misc/vmw_vmci/vmci_guest.c:102:14: sparse: sparse: symbol
'vmci_read_reg' was not declared. Should it be static?
drivers/misc/vmw_vmci/vmci_guest.c:112:44: sparse: sparse: incorrect type in
argument 2 (different address spaces) @@ expected void volatile [noderef]
__iomem *addr @@ got char * @@
drivers/misc/vmw_vmci/vmci_guest.c:112:44: sparse: expected void
volatile [noderef] __iomem *addr
drivers/misc/vmw_vmci/vmci_guest.c:112:44: sparse: got char *
drivers/misc/vmw_vmci/vmci_guest.c:109:6: sparse: sparse: symbol
'vmci_write_reg' was not declared. Should it be static?
drivers/misc/vmw_vmci/vmci_guest.c:514:27: sparse: sparse: incorrect type in
assignment (different address spaces) @@ expected char *mmio_base @@
got void [noderef] __iomem * @@
drivers/misc/vmw_vmci/vmci_guest.c:514:27: sparse: expected char
*mmio_base
drivers/misc/vmw_vmci/vmci_guest.c:514:27: sparse: got void [noderef]
__iomem *
>> drivers/misc/vmw_vmci/vmci_guest.c:869:43: sparse: sparse: incorrect type in
>> argument 2 (different address spaces) @@ expected void [noderef] __iomem
>> * @@ got char *mmio_base @@
drivers/misc/vmw_vmci/vmci_guest.c:869:43: sparse: expected void
[noderef] __iomem *
drivers/misc/vmw_vmci/vmci_guest.c:869:43: sparse: got char *mmio_base
vim +869 drivers/misc/vmw_vmci/vmci_guest.c
813
814 static void vmci_guest_remove_device(struct pci_dev *pdev)
815 {
816 struct vmci_guest_device *vmci_dev = pci_get_drvdata(pdev);
817 int vmci_err;
818
819 dev_dbg(>dev, "Removing device\n");
820
821 atomic_dec(_num_guest_devices);
822
823 vmci_qp_guest_endpoints_exit();
824
825 vmci_err = vmci_event_unsubscribe(ctx_update_sub_id);
826 if (vmci_err < VMCI_SUCCESS)
827 dev_warn(>dev,
828 "Failed to unsubscribe from event (type=%d)
with subscriber (ID=0x%x): %d\n",
829 VMCI_EVENT_CTX_ID_UPDATE, ctx_update_sub_id,
vmci_err);
830
831 spin_lock_irq(_dev_spinlock);
832 vmci_dev_g = NULL;
833 vmci_pdev = NULL;
834 spin_unlock_irq(_dev_spinlock);
835
836 dev_dbg(>dev, "Resetting vmci device\n");
837 vmci_write_reg(vmci_dev, VMCI_CONTROL_RESET, VMCI_CONTROL_ADDR);
838
839 /*
840 * Free IRQ and then disable MSI/MSI-X as appropriate. For
841 * MSI-X, we might have multiple vectors, each with their own
842 * IRQ, which we must free too.
843 */
844 if (vmci_dev->exclusive_vectors) {
845 free_irq(pci_irq_vector(pdev, 1), vmci_dev);
846 if (vmci_dev->mmio_base != NULL)
847 free_irq(pci_irq_vector(pdev, 2), vmci_dev);
848 }
849 free_irq(pci_irq_vector(pdev, 0), vmci_dev);
850
[PATCH 6/8] VMCI: dma dg: allocate send and receive buffers for DMA datagrams
If DMA datagrams are used, allocate send and receive buffers
in coherent DMA memory.
This is done in preparation for the send and receive datagram
operations, where the buffers are used for the exchange of data
between driver and device.
Reviewed-by: Vishnu Dasa
Signed-off-by: Jorgen Hansen
---
drivers/misc/vmw_vmci/vmci_guest.c | 66 ++
include/linux/vmw_vmci_defs.h | 4 ++
2 files changed, 63 insertions(+), 7 deletions(-)
diff --git a/drivers/misc/vmw_vmci/vmci_guest.c
b/drivers/misc/vmw_vmci/vmci_guest.c
index 1903fe8e7c68..d0acb686b464 100644
--- a/drivers/misc/vmw_vmci/vmci_guest.c
+++ b/drivers/misc/vmw_vmci/vmci_guest.c
@@ -31,6 +31,12 @@
#define VMCI_UTIL_NUM_RESOURCES 1
+/*
+ * Datagram buffers for DMA send/receive must accommodate at least
+ * a maximum sized datagram and the header.
+ */
+#define VMCI_DMA_DG_BUFFER_SIZE (VMCI_MAX_DG_SIZE + PAGE_SIZE)
+
static bool vmci_disable_msi;
module_param_named(disable_msi, vmci_disable_msi, bool, 0);
MODULE_PARM_DESC(disable_msi, "Disable MSI use in driver - (default=0)");
@@ -53,6 +59,9 @@ struct vmci_guest_device {
struct tasklet_struct bm_tasklet;
void *data_buffer;
+ dma_addr_t data_buffer_base;
+ void *tx_buffer;
+ dma_addr_t tx_buffer_base;
void *notification_bitmap;
dma_addr_t notification_base;
};
@@ -451,6 +460,24 @@ static irqreturn_t vmci_interrupt_dma_datagram(int irq,
void *_dev)
return IRQ_HANDLED;
}
+static void vmci_free_dg_buffers(struct vmci_guest_device *vmci_dev)
+{
+ if (vmci_dev->mmio_base != NULL) {
+ if (vmci_dev->tx_buffer != NULL)
+ dma_free_coherent(vmci_dev->dev,
+ VMCI_DMA_DG_BUFFER_SIZE,
+ vmci_dev->tx_buffer,
+ vmci_dev->tx_buffer_base);
+ if (vmci_dev->data_buffer != NULL)
+ dma_free_coherent(vmci_dev->dev,
+ VMCI_DMA_DG_BUFFER_SIZE,
+ vmci_dev->data_buffer,
+ vmci_dev->data_buffer_base);
+ } else {
+ vfree(vmci_dev->data_buffer);
+ }
+}
+
/*
* Most of the initialization at module load time is done here.
*/
@@ -520,11 +547,27 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
tasklet_init(_dev->bm_tasklet,
vmci_process_bitmap, (unsigned long)vmci_dev);
- vmci_dev->data_buffer = vmalloc(VMCI_MAX_DG_SIZE);
+ if (mmio_base != NULL) {
+ vmci_dev->tx_buffer = dma_alloc_coherent(>dev,
VMCI_DMA_DG_BUFFER_SIZE,
+
_dev->tx_buffer_base,
+GFP_KERNEL);
+ if (!vmci_dev->tx_buffer) {
+ dev_err(>dev,
+ "Can't allocate memory for datagram tx
buffer\n");
+ return -ENOMEM;
+ }
+
+ vmci_dev->data_buffer = dma_alloc_coherent(>dev,
VMCI_DMA_DG_BUFFER_SIZE,
+
_dev->data_buffer_base,
+ GFP_KERNEL);
+ } else {
+ vmci_dev->data_buffer = vmalloc(VMCI_MAX_DG_SIZE);
+ }
if (!vmci_dev->data_buffer) {
dev_err(>dev,
"Can't allocate memory for datagram buffer\n");
- return -ENOMEM;
+ error = -ENOMEM;
+ goto err_free_data_buffers;
}
pci_set_master(pdev); /* To enable queue_pair functionality. */
@@ -542,7 +585,7 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
if (!(capabilities & VMCI_CAPS_DATAGRAM)) {
dev_err(>dev, "Device does not support datagrams\n");
error = -ENXIO;
- goto err_free_data_buffer;
+ goto err_free_data_buffers;
}
caps_in_use = VMCI_CAPS_DATAGRAM;
@@ -586,7 +629,7 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
dev_err(>dev,
"Missing capability: VMCI_CAPS_DMA_DATAGRAM\n");
error = -ENXIO;
- goto err_free_data_buffer;
+ goto err_free_data_buffers;
}
}
@@ -602,6 +645,12 @@ static int vmci_guest_probe_device(struct pci_dev *pdev,
vmci_write_reg(vmci_dev, PAGE_SHIFT, VMCI_GUEST_PAGE_SHIFT);
page_shift = vmci_read_reg(vmci_dev, VMCI_GUEST_PAGE_SHIFT);
dev_info(>dev, "Using page shift %d\n", page_shift);
+
+ /* Configure the high order parts of the data in/out buffers. */
+ vmci_write_reg(vmci_dev,
