[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BY2PR0301MB071168CE194C74BFE98366A7A04B0@BY2PR0301MB0711.namprd03.prod.outlook.com>
Date: Tue, 20 Jan 2015 21:08:46 +0000
From: KY Srinivasan <kys@...rosoft.com>
To: Jake Oshins <jakeo@...rosoft.com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
"olaf@...fle.de" <olaf@...fle.de>,
"apw@...onical.com" <apw@...onical.com>,
"vkuznets@...hat.com" <vkuznets@...hat.com>
CC: Jake Oshins <jakeo@...rosoft.com>
Subject: RE: [PATCH 1/1] drivers:hv:vmbus Allow for more than one MMIO range
for children.
> -----Original Message-----
> From: Jake Oshins [mailto:jakeo@...rosoft.com]
> Sent: Tuesday, January 20, 2015 2:00 PM
> To: gregkh@...uxfoundation.org; KY Srinivasan; linux-
> kernel@...r.kernel.org; devel@...uxdriverproject.org; olaf@...fle.de;
> apw@...onical.com; vkuznets@...hat.com
> Cc: Jake Oshins
> Subject: [PATCH 1/1] drivers:hv:vmbus Allow for more than one MMIO range
> for children.
Include a descriptive commit log for this patch.
>
> Signed-off-by: Jake Oshins <jakeo@...rosoft.com>
> ---
> drivers/hv/vmbus_drv.c | 85
> +++++++++++++++++++++++++++++++++--------
> drivers/video/fbdev/hyperv_fb.c | 2 +-
> include/linux/hyperv.h | 2 +-
> 3 files changed, 72 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index
> 4d6b269..ba5b7a1 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -43,10 +43,7 @@ static struct tasklet_struct msg_dpc; static struct
> completion probe_event; static int irq;
>
> -struct resource hyperv_mmio = {
> - .name = "hyperv mmio",
> - .flags = IORESOURCE_MEM,
> -};
> +struct resource *hyperv_mmio;
> EXPORT_SYMBOL_GPL(hyperv_mmio);
>
> static int vmbus_exists(void)
> @@ -849,23 +846,74 @@ void vmbus_device_unregister(struct hv_device
> *device_obj)
>
>
> /*
> - * VMBUS is an acpi enumerated device. Get the the information we
> - * need from DSDT.
> + * VMBUS is an acpi enumerated device. Get the
> + * information we need from DSDT.
> */
>
> static acpi_status vmbus_walk_resources(struct acpi_resource *res, void
> *ctx) {
> + resource_size_t start = 0;
> + resource_size_t end = 0;
> + struct resource *new_res;
> + struct resource **old_res = &hyperv_mmio;
> +
> switch (res->type) {
> case ACPI_RESOURCE_TYPE_IRQ:
> irq = res->data.irq.interrupts[0];
> + return AE_OK;
> +
> + /*
> + * "Address" descriptors are for bus windows. Ignore
> + * "memory" descriptors, which are for registers on
> + * devices.
> + */
> + case ACPI_RESOURCE_TYPE_ADDRESS32:
> + start = res->data.address32.minimum;
> + end = res->data.address32.maximum;
> break;
>
> case ACPI_RESOURCE_TYPE_ADDRESS64:
> - hyperv_mmio.start = res->data.address64.minimum;
> - hyperv_mmio.end = res->data.address64.maximum;
> + start = res->data.address64.minimum;
> + end = res->data.address64.maximum;
> break;
> +
> + default:
> + /* Unused resource type */
> + return AE_OK;
> }
>
> + /*
> + * Ignore ranges that are below 1MB, as they're not
> + * necessary or useful here.
> + */
> + if (end < 0x100000)
> + return AE_OK;
> +
> + new_res = kzalloc(sizeof(*new_res), GFP_ATOMIC);
> + if (!new_res)
> + return AE_NO_MEMORY;
> +
> + new_res->name = "hyperv mmio";
> + new_res->flags = IORESOURCE_MEM;
> + new_res->start = start;
> + new_res->end = end;
> +
> + do {
> + if (!*old_res) {
> + *old_res = new_res;
> + break;
> + }
> +
> + if ((*old_res)->start > new_res->end) {
> + new_res->sibling = *old_res;
> + *old_res = new_res;
> + break;
> + }
> +
> + old_res = &(*old_res)->sibling;
> +
> + } while (1);
> +
> return AE_OK;
> }
>
> @@ -873,6 +921,7 @@ static int vmbus_acpi_add(struct acpi_device *device)
> {
> acpi_status result;
> int ret_val = -ENODEV;
> + struct acpi_device *ancestor;
>
> hv_acpi_dev = device;
>
> @@ -882,18 +931,22 @@ static int vmbus_acpi_add(struct acpi_device
> *device)
> if (ACPI_FAILURE(result))
> goto acpi_walk_err;
> /*
> - * The parent of the vmbus acpi device (Gen2 firmware) is the VMOD
> that
> - * has the mmio ranges. Get that.
> + * Some ancestor of the vmbus acpi device (Gen1 or Gen2
> + * firmware) is the VMOD that has the mmio ranges. Get that.
> */
> - if (device->parent) {
> - result = acpi_walk_resources(device->parent->handle,
> + for (ancestor = device->parent;
> + ancestor;
> + ancestor = ancestor->parent) {
> + result = acpi_walk_resources(ancestor->handle,
> METHOD_NAME__CRS,
> vmbus_walk_resources, NULL);
>
> if (ACPI_FAILURE(result))
> - goto acpi_walk_err;
> - if (hyperv_mmio.start && hyperv_mmio.end)
> - request_resource(&iomem_resource,
> &hyperv_mmio);
> + continue;
> + if (hyperv_mmio) {
> + request_resource(&iomem_resource,
> hyperv_mmio);
> + break;
> + }
> }
> ret_val = 0;
>
If we have a failure in walking the ACPI resources, you are not rolling back the
State with regards to the memory you allocated for the resource structure and the
resource you have requested.
> @@ -962,6 +1015,8 @@ static void __exit vmbus_exit(void)
> hv_remove_vmbus_irq();
> vmbus_free_channels();
> bus_unregister(&hv_bus);
> + if (hyperv_mmio)
> + release_resource(hyperv_mmio);
You are leaking memory here; you have not freed up the resource structures
that you allocated earlier.
> hv_cleanup();
> acpi_bus_unregister_driver(&vmbus_acpi_driver);
> }
> diff --git a/drivers/video/fbdev/hyperv_fb.c
> b/drivers/video/fbdev/hyperv_fb.c index 4254336..003c8f0 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -686,7 +686,7 @@ static int hvfb_getmem(struct fb_info *info)
> par->mem.name = KBUILD_MODNAME;
> par->mem.flags = IORESOURCE_MEM | IORESOURCE_BUSY;
> if (gen2vm) {
> - ret = allocate_resource(&hyperv_mmio, &par->mem,
> + ret = allocate_resource(hyperv_mmio, &par->mem,
> screen_fb_size,
> 0, -1,
> screen_fb_size,
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index
> 476c685..8903689 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -1178,7 +1178,7 @@ int hv_vss_init(struct hv_util_service *); void
> hv_vss_deinit(void); void hv_vss_onchannelcallback(void *);
>
> -extern struct resource hyperv_mmio;
> +extern struct resource *hyperv_mmio;
>
> /*
> * Negotiated version with the Host.
> --
> 1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists