Re: [Xen-devel] [XEN PATCH for-4.13 v1] Reset iomem's gfn to LIBXL_INVALID_GFN on reboot

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Oleksandr Grytsov writes ("[PATCH v1] 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.

Thanks for this patch.

I confess that I am not sure what is going on here.  Where is this
troublesome 0 coming from ?  I see that the default value for gfn in
the struct is 0 and looking for assignments before this patch, gfn is
defaulted from b_info->iomem[i].start, which is presumably non-0.

I suspect that your patch may be fixing this the wrong way.  I have
addressed this mail to various people who have touched this area of
code and hope they will be able to clarify.

BTW, please do ping this (and your other patches) by email, if the
conversation seems to stall.

Thanks,
Ian.

> 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 9d0eb5aed1..0ae16a5b12 100644
> --- a/tools/libxl/libxl_domain.c
> +++ b/tools/libxl/libxl_domain.c
> @@ -2120,6 +2120,15 @@ static void 
> retrieve_domain_configuration_end(libxl__egc *egc,
>          }
>      }
>  
> +    /* 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;
> +    }
> +
>      /* Devices: disk, nic, vtpm, pcidev etc. */
>  
>      /* The MERGE macro implements following logic:
> -- 
> 2.17.1
> 

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