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

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

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