* [PATCH 1/1] media: ipu6: Fix vmalloc memory allocation
2024-05-01 0:22 linux-next: build failure after merge of the v4l-dvb-next tree Stephen Rothwell
@ 2024-05-02 6:15 ` Sakari Ailus
2024-05-02 10:03 ` Mauro Carvalho Chehab
2024-05-02 15:49 ` [PATCH 1/1] media: intel/ipu6: Don't re-allocate memory for firmware Sakari Ailus
1 sibling, 1 reply; 4+ messages in thread
From: Sakari Ailus @ 2024-05-02 6:15 UTC (permalink / raw)
To: Stephen Rothwell, linux-media
Cc: Mauro Carvalho Chehab, Bingbu Cao, Hans Verkuil,
Linux Kernel Mailing List, Linux Next Mailing List
The driver was calling vmalloc() to allocate memory, something which isn't
available except when particular Kconfig settings are enabled.
Use kvmalloc() instead.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Fixes: 25fedc021985 ("media: intel/ipu6: add Intel IPU6 PCI device driver")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Hi Stephen,
Thanks for reporting this. I'm a bit surprised this wasn't catched
earlier. But it seems vmalloc() is defined in some configuration.
- Sakari
drivers/media/pci/intel/ipu6/ipu6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/pci/intel/ipu6/ipu6.c b/drivers/media/pci/intel/ipu6/ipu6.c
index 4b1f69d14d71..082b1d6196be 100644
--- a/drivers/media/pci/intel/ipu6/ipu6.c
+++ b/drivers/media/pci/intel/ipu6/ipu6.c
@@ -526,7 +526,7 @@ static int request_cpd_fw(const struct firmware **firmware_p, const char *name,
}
dst->size = fw->size;
- dst->data = vmalloc(fw->size);
+ dst->data = kvmalloc(fw->size, GFP_KERNEL);
if (!dst->data) {
kfree(dst);
ret = -ENOMEM;
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 1/1] media: intel/ipu6: Don't re-allocate memory for firmware
2024-05-01 0:22 linux-next: build failure after merge of the v4l-dvb-next tree Stephen Rothwell
2024-05-02 6:15 ` [PATCH 1/1] media: ipu6: Fix vmalloc memory allocation Sakari Ailus
@ 2024-05-02 15:49 ` Sakari Ailus
1 sibling, 0 replies; 4+ messages in thread
From: Sakari Ailus @ 2024-05-02 15:49 UTC (permalink / raw)
To: Stephen Rothwell, linux-media
Cc: Mauro Carvalho Chehab, Bingbu Cao, Hans Verkuil,
Linux Kernel Mailing List, Linux Next Mailing List
The ipu6 driver allocated vmalloc memory for the firmware if
request_firmware() somehow managed not to use vmalloc to allocate it.
Still how the memory is allocated by request_firmware() is not specified
in its API, so be prepared for kmalloc-allocated firmware, too. Instead of
allocating new vmalloc-backed buffer for the firmware, obtain the pages
from virtual addresses instead.
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Fixes: 25fedc021985 ("media: intel/ipu6: add Intel IPU6 PCI device driver")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
Hello everyone,
Mauro preferred not to merge the earlier patch. Admittedly, there are
better ways to fix the problem. Such as this one.
- Sakari
drivers/media/pci/intel/ipu6/ipu6-buttress.c | 7 +++-
drivers/media/pci/intel/ipu6/ipu6.c | 41 +-------------------
2 files changed, 7 insertions(+), 41 deletions(-)
diff --git a/drivers/media/pci/intel/ipu6/ipu6-buttress.c b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
index dbcf1aa87872..23c537e7ce1e 100644
--- a/drivers/media/pci/intel/ipu6/ipu6-buttress.c
+++ b/drivers/media/pci/intel/ipu6/ipu6-buttress.c
@@ -552,12 +552,16 @@ int ipu6_buttress_reset_authentication(struct ipu6_device *isp)
int ipu6_buttress_map_fw_image(struct ipu6_bus_device *sys,
const struct firmware *fw, struct sg_table *sgt)
{
+ bool is_vmalloc = is_vmalloc_addr(fw->data);
struct page **pages;
const void *addr;
unsigned long n_pages;
unsigned int i;
int ret;
+ if (!is_vmalloc && !virt_addr_valid(fw->data))
+ return -EDOM;
+
n_pages = PHYS_PFN(PAGE_ALIGN(fw->size));
pages = kmalloc_array(n_pages, sizeof(*pages), GFP_KERNEL);
@@ -566,7 +570,8 @@ int ipu6_buttress_map_fw_image(struct ipu6_bus_device *sys,
addr = fw->data;
for (i = 0; i < n_pages; i++) {
- struct page *p = vmalloc_to_page(addr);
+ struct page *p = is_vmalloc ?
+ vmalloc_to_page(addr) : virt_to_page(addr);
if (!p) {
ret = -ENOMEM;
diff --git a/drivers/media/pci/intel/ipu6/ipu6.c b/drivers/media/pci/intel/ipu6/ipu6.c
index 7bcd9c5a381a..2cf04251c9e7 100644
--- a/drivers/media/pci/intel/ipu6/ipu6.c
+++ b/drivers/media/pci/intel/ipu6/ipu6.c
@@ -503,45 +503,6 @@ static void ipu6_configure_vc_mechanism(struct ipu6_device *isp)
writel(val, isp->base + BUTTRESS_REG_BTRS_CTRL);
}
-static int request_cpd_fw(const struct firmware **firmware_p, const char *name,
- struct device *device)
-{
- const struct firmware *fw;
- struct firmware *dst;
- int ret = 0;
-
- ret = request_firmware(&fw, name, device);
- if (ret)
- return ret;
-
- if (is_vmalloc_addr(fw->data)) {
- *firmware_p = fw;
- return 0;
- }
-
- dst = kzalloc(sizeof(*dst), GFP_KERNEL);
- if (!dst) {
- ret = -ENOMEM;
- goto release_firmware;
- }
-
- dst->size = fw->size;
- dst->data = vmalloc(fw->size);
- if (!dst->data) {
- kfree(dst);
- ret = -ENOMEM;
- goto release_firmware;
- }
-
- memcpy((void *)dst->data, fw->data, fw->size);
- *firmware_p = dst;
-
-release_firmware:
- release_firmware(fw);
-
- return ret;
-}
-
static int ipu6_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct ipu6_buttress_ctrl *isys_ctrl = NULL, *psys_ctrl = NULL;
@@ -627,7 +588,7 @@ static int ipu6_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
if (ret)
return ret;
- ret = request_cpd_fw(&isp->cpd_fw, isp->cpd_fw_name, dev);
+ ret = request_firmware(&isp->cpd_fw, isp->cpd_fw_name, dev);
if (ret) {
dev_err_probe(&isp->pdev->dev, ret,
"Requesting signed firmware %s failed\n",
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread