Re: [Xen-devel] Problem with IOMEM and domain reboot

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Dec 20, 2017 at 06:27:02PM +0200, Oleksandr Andrushchenko wrote:
> Hi, all!
> 
> While trying to reboot a domain which has iomem configured
> (we are passing through some devices), I found an issue,
> that after domain reboot those iomem's are incorrectly re-mapped,
> e.g. for the configuration snippet below fe960 -> 0.
> 
> Part of the domain config I use:
> iomem=[
>     "0xfd010,1@0xfd000",
>     "fe960,8",
> ]
> 
> During domain creation:
> libxl_create.c:210:libxl__domain_build_info_setdefault: iomem gfn fd000
> start fd010
> libxl_create.c:210:libxl__domain_build_info_setdefault: iomem gfn
> ffffffffffffffff start fe960
> 
> which means that for fe960 initial value was set to LIBXL_INVALID_GFN
> and then on domain configuration,
> tools/libxl/libxl_create.c:libxl__domain_build_info_setdefault:
> 
>     for (i = 0 ; i < b_info->num_iomem; i++)
>         if (b_info->iomem[i].gfn == LIBXL_INVALID_GFN)
>             b_info->iomem[i].gfn = b_info->iomem[i].start;
> 
> made that GFN for fe960 to be set to the correct value.
> 
> But during domain reboot I see that
> tools/xl/xl_vmcontrol.c:reload_domain_config
> tries to replicate configuration from the original domain config
> being rebooted, but that leads to iomem's GFN to be set to 0 (if configured
> in form [IOMEM_START,NUM_PAGES], but for [IOMEM_START,NUM_PAGES[@GFN] it is
> ok):
> 
> iomem gfn fd000 start fd010
> iomem gfn 0 start fe960
> 
> Thus, further domain restart procedure leads to invalid mapping, e.g. fe960
> -> 0.
> 
> I created a patch which allowed me to reboot the domain, but I would love
> to hear comments on what would be the proper fix.
> 
> Thank you,
> Oleksandr
> 

> From aa1f20af73a5a3c8f2c904b857a79334d18d41ff Mon Sep 17 00:00:00 2001
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> Date: Wed, 20 Dec 2017 17:51:18 +0200
> Subject: [PATCH] [HACK] Reset iomem's gfn to LIBXL_INVALID_GFN on reboot
> 
> During domain reboot its configuration is partially reused
> to re-create a new domain, but iomem's GFN field for the
> iomem is only restored for those memory ranges, which are
> configured in form of [IOMEM_START,NUM_PAGES[@GFN], but not for
> those in form of [IOMEM_START,NUM_PAGES], e.g. without GFN.
> For the latter GFN is reset to 0, but while mapping ranges
> to a domain during reboot there is a check that GFN treated
> as valid if it is not equal to LIBXL_INVALID_GFN, thus making
> Xen to map IOMEM_START to address 0 in the guest's address space.
> 
> Workaround it by resseting GFN to LIBXL_INVALID_GFN, so xl
> can set proper values for mapping on reboot.
> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> ---
>  tools/libxl/libxl_domain.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tools/libxl/libxl_domain.c b/tools/libxl/libxl_domain.c
> index ef1a0927b00d..2678ad2ad54f 100644
> --- a/tools/libxl/libxl_domain.c
> +++ b/tools/libxl/libxl_domain.c
> @@ -1647,6 +1647,15 @@ int libxl_retrieve_domain_configuration(libxl_ctx 
> *ctx, uint32_t domid,
>          }
>      }
>  
> +    /* reset IOMEM's GFN to initial value */
> +    {
> +        int i;
> +
> +        for (i = 0; i < d_config->b_info.num_iomem; i++)
> +            if (d_config->b_info.iomem[i].gfn == 0)
> +                d_config->b_info.iomem[i].gfn = LIBXL_INVALID_GFN;
> +    }
> +

I don't think this is necessary. Instead we should tell libxl to save
the generated value into the template. Add an update_config hook for the
iomem type should be better.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel