QEMU-Devel Archive mirror
 help / color / mirror / Atom feed
From: Peter Maydell <peter.maydell@linaro.org>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: incoming+qemu-project-qemu-11167699-3xhw7c0pviow7og92yv73e0tr-issue-404@incoming.gitlab.com,
	Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
	Richard Henderson <richard.henderson@linaro.org>,
	QEMU devel list <qemu-devel@nongnu.org>,
	Programmingkid <programmingkidx@gmail.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: tb_flush() calls causing long Windows XP boot times
Date: Wed, 16 Jun 2021 14:06:04 +0100	[thread overview]
Message-ID: <CAFEAcA-PafgiZWOi3TvM1VRpfKVRn+L-7bu037nLocSsXNtEzQ@mail.gmail.com> (raw)
In-Reply-To: <874kdygedv.fsf@linaro.org>

On Wed, 16 Jun 2021 at 13:53, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> writes:
> > diff --git a/exec.c b/exec.c
> > index 67e520d18e..7f4074f95e 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -1019,14 +1019,13 @@ void tb_invalidate_phys_addr(AddressSpace *as,
> > hwaddr addr, MemTxAttrs attrs)
> >
> >  static void breakpoint_invalidate(CPUState *cpu, target_ulong pc)
> >  {
> > -    MemTxAttrs attrs;
> > -    hwaddr phys = cpu_get_phys_page_attrs_debug(cpu, pc, &attrs);
> > -    int asidx = cpu_asidx_from_attrs(cpu, attrs);
> > -    if (phys != -1) {
> > -        /* Locks grabbed by tb_invalidate_phys_addr */
> > -        tb_invalidate_phys_addr(cpu->cpu_ases[asidx].as,
> > -                                phys | (pc & ~TARGET_PAGE_MASK), attrs);
> > -    }
> > +    /*
> > +     * There may not be a virtual to physical translation for the pc
> > +     * right now, but there may exist cached TB for this pc.
> > +     * Flush the whole TB cache to force re-translation of such TBs.
> > +     * This is heavyweight, but we're debugging anyway.
> > +     */
> > +    tb_flush(cpu);
> >  }
> >  #endif
> >
> >
> > Unfortunately my x86-fu isn't really enough to understand what the
> > solution should be in this case.
>
> It's not really an x86 issue here but that we don't have any easy way of
> finding the subset of TranslationBlock's that might be affected. We can
> only query the QHT for a head address + flags. Meanwhile when there is
> an active mapping we go through the page tables

Could we do something where we zap the TBs here where there is an active
virtual-to-physical mapping for this PC, and also make a record of affected
PCs (or PC ranges) so that before we add a new entry to the
virtual-to-physical mapping we check the record to see if we actually need
to flush this TB? I think if you flush all the TLBs at this point then
you can do the "check before adding new entry" part in
tlb_set_page_with_attrs(),
but I'm not super familiar with the execution flow of TCG so that might be
wrong. Also there needs to be a point where we can discard entries from
our "dump this TB for this PC" records so they don't just grow indefinitely,
and I'm not sure what that would be.

thanks
-- PMM


  reply	other threads:[~2021-06-16 13:08 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-10 12:59 tb_flush() calls causing long Windows XP boot times Programmingkid
2021-06-10 13:14 ` Peter Maydell
2021-06-10 13:24   ` Mark Cave-Ayland
2021-06-11 11:24     ` Alex Bennée
2021-06-11 15:01       ` Programmingkid
2021-06-11 17:13         ` Paolo Bonzini
2021-06-11 18:22           ` Alex Bennée
2021-06-13 14:03             ` Mark Cave-Ayland
2021-06-14 14:37               ` Alex Bennée
2021-06-15 13:58                 ` Programmingkid
2021-06-16  1:58                   ` Richard Henderson
2021-06-16  8:59                     ` Mark Cave-Ayland
2021-06-16 12:53                       ` Alex Bennée
2021-06-16 13:06                         ` Peter Maydell [this message]
2021-06-16 15:30                           ` Alex Bennée
2021-06-16 13:21                       ` Alex Bennée
2021-06-16 12:12                     ` Programmingkid
2021-06-10 13:38   ` Programmingkid
2021-06-14 22:19 ` no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFEAcA-PafgiZWOi3TvM1VRpfKVRn+L-7bu037nLocSsXNtEzQ@mail.gmail.com \
    --to=peter.maydell@linaro.org \
    --cc=alex.bennee@linaro.org \
    --cc=incoming+qemu-project-qemu-11167699-3xhw7c0pviow7og92yv73e0tr-issue-404@incoming.gitlab.com \
    --cc=mark.cave-ayland@ilande.co.uk \
    --cc=pbonzini@redhat.com \
    --cc=programmingkidx@gmail.com \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).