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

[Julien Grall <julien.grall@xxxxxxx>]

Hi Ian,

On 11/10/2019 16:14, Ian Jackson wrote:
> 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.
I found a thread from December 2017 related to the problem described here [1].

Looking at the thread, there were no conclusion on the root causes and some 
questions were left unanswered by the contributor (see [2]).
Oleksandr, could you look at the thread and see if you can provide more details 
what's going on? Answering back here would be fine.
> 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
Cheers,

[1] <ebf78aec-dcfd-72d9-dac2-06b29e4a66ae@xxxxxxxxx>
[2] <20180213122432.h4fh22ej4dfe7226@xxxxxxxxxx>

--
Julien Grall

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