Hi, On 10/30/2015 09:51 AM, Baptiste Reynal wrote: > Hi James, > > Thanks for this fix. > > Acked-by: Baptiste Reynal <b.reynal@xxxxxxxxxxxxxxxxxxxxxx> > Tested-by: Baptiste Reynal <b.reynal@xxxxxxxxxxxxxxxxxxxxxx> > > On Thu, Oct 29, 2015 at 5:50 PM, James Morse <james.morse@xxxxxxx> wrote: >> vfio_platform_{read,write}_mmio() call ioremap_nocache() to map >> a region of io memory, which they store in struct vfio_platform_region to >> be eventually re-used, or unmapped by vfio_platform_regions_cleanup(). >> >> These functions receive a copy of their struct vfio_platform_region >> argument on the stack - so these mapped areas are always allocated, and >> always leaked. I just noticed I have a leak in reset modules too. I am going to correct this. Thanks Eric >> >> Pass this argument as a pointer instead. >> >> Fixes: 6e3f26456009 "vfio/platform: read and write support for the device fd" >> Signed-off-by: James Morse <james.morse@xxxxxxx> >> --- >> drivers/vfio/platform/vfio_platform_common.c | 36 ++++++++++++++-------------- >> 1 file changed, 18 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/vfio/platform/vfio_platform_common.c b/drivers/vfio/platform/vfio_platform_common.c >> index f3b6299..ccf5da5 100644 >> --- a/drivers/vfio/platform/vfio_platform_common.c >> +++ b/drivers/vfio/platform/vfio_platform_common.c >> @@ -308,17 +308,17 @@ static long vfio_platform_ioctl(void *device_data, >> return -ENOTTY; >> } >> >> -static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, >> +static ssize_t vfio_platform_read_mmio(struct vfio_platform_region *reg, >> char __user *buf, size_t count, >> loff_t off) >> { >> unsigned int done = 0; >> >> - if (!reg.ioaddr) { >> - reg.ioaddr = >> - ioremap_nocache(reg.addr, reg.size); >> + if (!reg->ioaddr) { >> + reg->ioaddr = >> + ioremap_nocache(reg->addr, reg->size); >> >> - if (!reg.ioaddr) >> + if (!reg->ioaddr) >> return -ENOMEM; >> } >> >> @@ -328,7 +328,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, >> if (count >= 4 && !(off % 4)) { >> u32 val; >> >> - val = ioread32(reg.ioaddr + off); >> + val = ioread32(reg->ioaddr + off); >> if (copy_to_user(buf, &val, 4)) >> goto err; >> >> @@ -336,7 +336,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, >> } else if (count >= 2 && !(off % 2)) { >> u16 val; >> >> - val = ioread16(reg.ioaddr + off); >> + val = ioread16(reg->ioaddr + off); >> if (copy_to_user(buf, &val, 2)) >> goto err; >> >> @@ -344,7 +344,7 @@ static ssize_t vfio_platform_read_mmio(struct vfio_platform_region reg, >> } else { >> u8 val; >> >> - val = ioread8(reg.ioaddr + off); >> + val = ioread8(reg->ioaddr + off); >> if (copy_to_user(buf, &val, 1)) >> goto err; >> >> @@ -377,7 +377,7 @@ static ssize_t vfio_platform_read(void *device_data, char __user *buf, >> return -EINVAL; >> >> if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO) >> - return vfio_platform_read_mmio(vdev->regions[index], >> + return vfio_platform_read_mmio(&vdev->regions[index], >> buf, count, off); >> else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO) >> return -EINVAL; /* not implemented */ >> @@ -385,17 +385,17 @@ static ssize_t vfio_platform_read(void *device_data, char __user *buf, >> return -EINVAL; >> } >> >> -static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, >> +static ssize_t vfio_platform_write_mmio(struct vfio_platform_region *reg, >> const char __user *buf, size_t count, >> loff_t off) >> { >> unsigned int done = 0; >> >> - if (!reg.ioaddr) { >> - reg.ioaddr = >> - ioremap_nocache(reg.addr, reg.size); >> + if (!reg->ioaddr) { >> + reg->ioaddr = >> + ioremap_nocache(reg->addr, reg->size); >> >> - if (!reg.ioaddr) >> + if (!reg->ioaddr) >> return -ENOMEM; >> } >> >> @@ -407,7 +407,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, >> >> if (copy_from_user(&val, buf, 4)) >> goto err; >> - iowrite32(val, reg.ioaddr + off); >> + iowrite32(val, reg->ioaddr + off); >> >> filled = 4; >> } else if (count >= 2 && !(off % 2)) { >> @@ -415,7 +415,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, >> >> if (copy_from_user(&val, buf, 2)) >> goto err; >> - iowrite16(val, reg.ioaddr + off); >> + iowrite16(val, reg->ioaddr + off); >> >> filled = 2; >> } else { >> @@ -423,7 +423,7 @@ static ssize_t vfio_platform_write_mmio(struct vfio_platform_region reg, >> >> if (copy_from_user(&val, buf, 1)) >> goto err; >> - iowrite8(val, reg.ioaddr + off); >> + iowrite8(val, reg->ioaddr + off); >> >> filled = 1; >> } >> @@ -453,7 +453,7 @@ static ssize_t vfio_platform_write(void *device_data, const char __user *buf, >> return -EINVAL; >> >> if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_MMIO) >> - return vfio_platform_write_mmio(vdev->regions[index], >> + return vfio_platform_write_mmio(&vdev->regions[index], >> buf, count, off); >> else if (vdev->regions[index].type & VFIO_PLATFORM_REGION_TYPE_PIO) >> return -EINVAL; /* not implemented */ >> -- >> 2.6.1 >> > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html