All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Jones <drjones@redhat.com>
To: Shih-Wei Li <shihwei@cs.columbia.edu>
Cc: Christoffer Dall <cdall@cs.columbia.edu>,
	kvm@vger.kernel.org, Marc Zyngier <marc.zyngier@arm.com>,
	Yury Norov <ynorov@caviumnetworks.com>,
	kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH kvm-unit-tests 2/2] arm64: add micro-bench
Date: Mon, 3 Sep 2018 18:31:39 +0200	[thread overview]
Message-ID: <20180903163139.arpf6vpg4d2z7mju@kamzik.brq.redhat.com> (raw)
In-Reply-To: <CAO+sbHHswsoUqhnwwzB5bGoc7G_cbkw24t6Br1=7bULVxoctaw@mail.gmail.com>

On Mon, Sep 03, 2018 at 11:06:51AM -0400, Shih-Wei Li wrote:
> On Thu, Aug 30, 2018 at 10:17 AM, Andrew Jones <drjones@redhat.com> wrote:
> > +static bool test_init(void)
> > +{
> > +       int v = gic_init();
> > +
> > +       if (!v) {
> > +               printf("No supported gic present, skipping tests...\n");
> > +               return false;
> > +       }
> > +
> > +       if (nr_cpus < 2) {
> > +               printf("At least two cpus required, skipping tests...\n");
> > +               return false;
> > +       }
> > +
> > +       switch (v) {
> > +       case 2:
> > +               vgic_dist_base = gicv2_dist_base();
> > +               write_eoir = gicv2_write_eoir;
> > +       case 3:
> > +               vgic_dist_base = gicv3_dist_base();
> > +               write_eoir = gicv3_write_eoir;
> > +       }
> 
> I think we'll need a "break" in the switch case body for gicv2 here
> otherwise it'll fall into the case body below and refer to the wrong
> symbols for gicv3.
> 
> As I've tested on my seattle server hardware (with gicv2), this results
> to incorrect execution in mmio_read_vgic_exec and a crash in
> eoi_exec().
>

Argh... Obviously my testing of this was pretty poor. You are certainly
right that I'm missing breaks in this switch. If you don't mind, please
add them, along with the fix in v2, to your local repo and test again.
I'll hold off on sending v3 until you've had a chance to test and review
more.

Thanks,
drew

  reply	other threads:[~2018-09-03 16:31 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-30 14:17 [PATCH kvm-unit-tests 0/2] Support micro operation measurement on arm64 Andrew Jones
2018-08-30 14:17 ` [PATCH kvm-unit-tests 1/2] arm/arm64: add GICD_IIDR definition Andrew Jones
2018-08-30 14:17 ` [PATCH kvm-unit-tests 2/2] arm64: add micro-bench Andrew Jones
2018-09-03 11:12   ` Christoffer Dall
2018-09-03 12:11     ` Andrew Jones
2018-09-03 13:14       ` Christoffer Dall
2018-09-03 14:59         ` Andrew Jones
2018-09-03 15:06   ` Shih-Wei Li
2018-09-03 16:31     ` Andrew Jones [this message]
2018-09-04  3:38       ` Shih-Wei Li
2018-09-04 15:02         ` Shih-Wei Li
2018-09-04 15:20           ` Andrew Jones

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=20180903163139.arpf6vpg4d2z7mju@kamzik.brq.redhat.com \
    --to=drjones@redhat.com \
    --cc=cdall@cs.columbia.edu \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=marc.zyngier@arm.com \
    --cc=shihwei@cs.columbia.edu \
    --cc=ynorov@caviumnetworks.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.