Re: [Xen-devel] [RFC 16/16] xen/arm: Track page accessed between batch of Set/Way operations

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 11/5/18 9:35 PM, Stefano Stabellini wrote:
> On Mon, 8 Oct 2018, Julien Grall wrote:
> > At the moment, the implementation of Set/Way operations will go through
> > all the entries of the guest P2M and flush them. However, this is very
> > expensive and may render unusable a guest OS using them.
> >
> > For instance, Linux 32-bit will use Set/Way operations during secondary
> > CPU bring-up. As the implementation is really expensive, it may be possible
> > to hit the CPU bring-up timeout.
> >
> > To limit the Set/Way impact, we track what pages has been of the guest
> > has been accessed between batch of Set/Way operations. This is done
> > using bit[0] (aka valid bit) of the P2M entry.
> This is going to improve performance of ill-mannered guests at the cost
> of hurting performance of well-mannered guests. Is it really a good
> trade-off? Should this behavior at least be configurable with a Xen
> command line?
Well, we have the choice between not been able to boot Linux 32-bit 
anymore or have a slight impact at the boot time for all guests.
As you may have noticed the command line is been suggested below. I 
didn't yet implemented as we agreed at Connect it would be good to start 
getting feedback on it.
> > This patch adds a new per-arch helper is introduced to perform actions just
> > before the guest is first unpaused. This will be used to invalidate the
> > P2M to track access from the start of the guest.
> >
> > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> >
> > ---
> >
> > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Cc: Julien Grall <julien.grall@xxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > Cc: Tim Deegan <tim@xxxxxxx>
> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> >   xen/arch/arm/domain.c       | 14 ++++++++++++++
> >   xen/arch/arm/domain_build.c |  7 +++++++
> >   xen/arch/arm/p2m.c          | 32 +++++++++++++++++++++++++++++++-
> >   xen/arch/x86/domain.c       |  4 ++++
> >   xen/common/domain.c         |  5 ++++-
> >   xen/include/asm-arm/p2m.h   |  2 ++
> >   xen/include/xen/domain.h    |  2 ++
> >   7 files changed, 64 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> > index feebbf5a92..f439f4657a 100644
> > --- a/xen/arch/arm/domain.c
> > +++ b/xen/arch/arm/domain.c
> > @@ -738,6 +738,20 @@ int arch_domain_soft_reset(struct domain *d)
> >       return -ENOSYS;
> >   }
> >   
> > +void arch_domain_creation_finished(struct domain *d)
> > +{
> > +    /*
> > +     * To avoid flushing the whole guest RAM on the first Set/Way, we
> > +     * invalidate the P2M to track what has been accessed.
> > +     *
> > +     * This is only turned when IOMMU is not used or the page-table are
> > +     * not shared because bit[0] (e.g valid bit) unset will result
> > +     * IOMMU fault that could be not fixed-up.
> > +     */
> > +    if ( !iommu_use_hap_pt(d) )
> > +        p2m_invalidate_root(p2m_get_hostp2m(d));
> > +}
> > +
> >   static int is_guest_pv32_psr(uint32_t psr)
> >   {
> >       switch (psr & PSR_MODE_MASK)
> > diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> > index f552154e93..de96516faa 100644
> > --- a/xen/arch/arm/domain_build.c
> > +++ b/xen/arch/arm/domain_build.c
> > @@ -2249,6 +2249,13 @@ int __init construct_dom0(struct domain *d)
> >       v->is_initialised = 1;
> >       clear_bit(_VPF_down, &v->pause_flags);
> >   
> > +    /*
> > +     * XXX: We probably want a command line option to invalidate or not
> > +     * the P2M. This is because invalidating the P2M will not work with
> > +     * IOMMU, however if this is not done there will be an impact.
> Why can't we check on iommu_use_hap_pt(d) like in
> arch_domain_creation_finished?
>
> In any case, I agree it is a good idea to introduce a command line
> parameter to toggle the p2m_invalidate_root call at domain creation
> on/off. There are cases where it should be off even if an IOMMU is
> present.
I actually forgot to remove that code as Dom0 should be covered by the 
change below.
I am not entirely to understand your last sentence, this feature is 
turned off when an IOMMU is present. So what is your use case here?
Cheers,

--
Julien Grall

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