lkml.org 
[lkml]   [2014]   [Jan]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH v13 08/19] xen/pvh/mmu: Use PV TLB instead of native.
    On Sun, Jan 05, 2014 at 06:11:39PM +0000, Stefano Stabellini wrote:
    > On Fri, 3 Jan 2014, Konrad Rzeszutek Wilk wrote:
    > > From: Mukesh Rathor <mukesh.rathor@oracle.com>
    > >
    > > We also optimize one - the TLB flush. The native operation would
    > > needlessly IPI offline VCPUs causing extra wakeups. Using the
    > > Xen one avoids that and lets the hypervisor determine which
    > > VCPU needs the TLB flush.
    > >
    > > Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
    > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
    > > ---
    > > arch/x86/xen/mmu.c | 9 +++++++++
    > > 1 file changed, 9 insertions(+)
    > >
    > > diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
    > > index 490ddb3..c1d406f 100644
    > > --- a/arch/x86/xen/mmu.c
    > > +++ b/arch/x86/xen/mmu.c
    > > @@ -2222,6 +2222,15 @@ static const struct pv_mmu_ops xen_mmu_ops __initconst = {
    > > void __init xen_init_mmu_ops(void)
    > > {
    > > x86_init.paging.pagetable_init = xen_pagetable_init;
    > > +
    > > + /* Optimization - we can use the HVM one but it has no idea which
    > > + * VCPUs are descheduled - which means that it will needlessly IPI
    > > + * them. Xen knows so let it do the job.
    > > + */
    > > + if (xen_feature(XENFEAT_auto_translated_physmap)) {
    > > + pv_mmu_ops.flush_tlb_others = xen_flush_tlb_others;
    > > + return;
    > > + }
    > > pv_mmu_ops = xen_mmu_ops;
    > >
    > > memset(dummy_mapping, 0xff, PAGE_SIZE);
    >
    > Regarding this patch, the next one and the other changes to
    > xen_setup_shared_info, xen_setup_mfn_list_list,
    > xen_setup_vcpu_info_placement, etc: considering that the mmu related
    > stuff is very different between PV and PVH guests, I wonder if it makes
    > any sense to keep calling xen_init_mmu_ops on PVH.
    >
    > I would introduce a new function, xen_init_pvh_mmu_ops, that sets
    > pv_mmu_ops.flush_tlb_others and only calls whatever is needed for PVH
    > under a new xen_pvh_pagetable_init.
    > Just to give you an idea, not even compiled tested:

    There is something to be said about sharing the same code path
    that "old-style" PV is using with the new-style - code coverage.

    That is the code gets tested under both platforms and if I (or
    anybody else) introduce a bug in the "common-PV-paths" it will
    be immediately obvious as hopefully the regression tests
    will pick it up.

    It is not nice - as low-level code is sprinkled with the one-offs
    for the PVH - which mostly is doing _less_.

    What I was thinking is to flip this around. Make the PVH paths
    the default and then have something like 'if (!xen_pvh_domain())'
    ... the big code.

    Would you be OK with this line of thinking going forward say
    after this patchset?

    Thanks!

    \
     
     \ /
      Last update: 2014-01-05 21:01    [W:3.529 / U:0.392 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site