On Mon, Sep 3, 2018 at 12:31 PM, Andrew Jones <drjones@xxxxxxxxxx> wrote:
> 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
I've added the break to the switch and it resolved the problems I
mentioned earlier. I'll do more testing and reviewing, and will get
back to you as soon as I can.
Thanks for formalizing the new patch set for micro-test!
Shih-Wei
