Re: [Xen-devel] [v8][PATCH 06/16] hvmloader/pci: disable all pci devices conflicting with rdm

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

> > Here what I intended to do is if one of all bars specific to one given
> > device already conflicts with RDM, its not necessary to continue check other
> > remaining bars of this device and other RDM regions, we just disable this
> > device simply then check next device.
> I know what you're trying to do; what I'm saying is I don't think it
> does what you want it to do.
>
> You have loops nested 3 deep:
> 1. for each dev
>    2.  for each bar
>      3. for each memory range
>
> This conditional is in loop 3; you want it to be in loop 2.
>
> (In fact, when you set is_conflict, you then break out of loop 3 back
> into loop 2; so this code will never actually be run.)
Sorry I should make this clear last time.

I mean I already knew what you were saying is right at this point so I 
tried to use goto to fix this bug.
>   >> Also, if you declare is_conflict inside the devfn loop, rather than in
> > > the main function, then you don't need this "is_conflict=false" here.
> > >
> > > It might also be more sensible to use a goto instead; but this is one
[snip]

> I'm not a fan of hard-coding the loop continuing condition like this;
> if I were going to do a goto, I'd want to go to the end of the loop.
I guess something like this,

                        ...
                        pci_writew(devfn, PCI_COMMAND, ~cmd);
                        /* Jump next device. */
                        goto check_next_device;
                    }
                }
            }
        }
 check_next_device:
    }
}

> Anyway, the code is OK as it is; I'd rather spend time working on
> something that's more of a blocker.
Thanks
Tiejun



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel