Re: [PATCH v2] xen/serial: Don't leak memory mapping if the serial initialization has failed

[Julien Grall <julien.grall@linaro.org>]

Ian, Keir: Ping?

On 02/19/2014 06:07 PM, Julien Grall wrote:
> The memory mapping is leaking when the serial driver failed to retrieve
> the IRQ. We can safely move the call to ioremap after.
> 
> Also use ioremap_cache instead of ioremap_attr in some serial drivers.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> 
> ---
> 
> Ian, I have dropped your ack because the patch fundamentaly changed.
> 
>     Changes in v2:
>         - s/ioremap_attr/ioremap_nocache
>         - Move ioremap call after retrieve the IRQ
> ---
>  xen/drivers/char/exynos4210-uart.c |   13 +++++++------
>  xen/drivers/char/omap-uart.c       |   15 ++++++++-------
>  xen/drivers/char/pl011.c           |   15 +++++++--------
>  3 files changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/xen/drivers/char/exynos4210-uart.c b/xen/drivers/char/exynos4210-uart.c
> index 0619575..150d49b 100644
> --- a/xen/drivers/char/exynos4210-uart.c
> +++ b/xen/drivers/char/exynos4210-uart.c
> @@ -334,12 +334,6 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
>          return res;
>      }
>  
> -    uart->regs = ioremap_nocache(addr, size);
> -    if ( !uart->regs )
> -    {
> -        early_printk("exynos4210: Unable to map the UART memory\n");
> -        return -ENOMEM;
> -    }
>      res = dt_device_get_irq(dev, 0, &uart->irq);
>      if ( res )
>      {
> @@ -347,6 +341,13 @@ static int __init exynos4210_uart_init(struct dt_device_node *dev,
>          return res;
>      }
>  
> +    uart->regs = ioremap_nocache(addr, size);
> +    if ( !uart->regs )
> +    {
> +        early_printk("exynos4210: Unable to map the UART memory\n");
> +        return -ENOMEM;
> +    }
> +
>      uart->vuart.base_addr = addr;
>      uart->vuart.size = size;
>      uart->vuart.data_off = UTXH;
> diff --git a/xen/drivers/char/omap-uart.c b/xen/drivers/char/omap-uart.c
> index c1580ef..b29f610 100644
> --- a/xen/drivers/char/omap-uart.c
> +++ b/xen/drivers/char/omap-uart.c
> @@ -326,13 +326,6 @@ static int __init omap_uart_init(struct dt_device_node *dev,
>          return res;
>      }
>  
> -    uart->regs = ioremap_attr(addr, size, PAGE_HYPERVISOR_NOCACHE);
> -    if ( !uart->regs )
> -    {
> -        early_printk("omap-uart: Unable to map the UART memory\n");
> -        return -ENOMEM;
> -    }
> -
>      res = dt_device_get_irq(dev, 0, &uart->irq);
>      if ( res )
>      {
> @@ -340,6 +333,14 @@ static int __init omap_uart_init(struct dt_device_node *dev,
>          return res;
>      }
>  
> +    uart->regs = ioremap_nocache(addr, size);
> +    if ( !uart->regs )
> +    {
> +        early_printk("omap-uart: Unable to map the UART memory\n");
> +        return -ENOMEM;
> +    }
> +
> +
>      uart->vuart.base_addr = addr;
>      uart->vuart.size = size;
>      uart->vuart.data_off = UART_THR;
> diff --git a/xen/drivers/char/pl011.c b/xen/drivers/char/pl011.c
> index fd82511..fe99af6 100644
> --- a/xen/drivers/char/pl011.c
> +++ b/xen/drivers/char/pl011.c
> @@ -248,14 +248,6 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
>          return res;
>      }
>  
> -    uart->regs = ioremap_attr(addr, size, PAGE_HYPERVISOR_NOCACHE);
> -    if ( !uart->regs )
> -    {
> -        early_printk("pl011: Unable to map the UART memory\n");
> -
> -        return -ENOMEM;
> -    }
> -
>      res = dt_device_get_irq(dev, 0, &uart->irq);
>      if ( res )
>      {
> @@ -263,6 +255,13 @@ static int __init pl011_uart_init(struct dt_device_node *dev,
>          return res;
>      }
>  
> +    uart->regs = ioremap_nocache(addr, size);
> +    if ( !uart->regs )
> +    {
> +        early_printk("pl011: Unable to map the UART memory\n");
> +        return -ENOMEM;
> +    }
> +
>      uart->vuart.base_addr = addr;
>      uart->vuart.size = size;
>      uart->vuart.data_off = DR;
> 


-- 
Julien Grall