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@xxxxxxxxxx> 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
