Re: [v7][PATCH 06/16] hvmloader/pci: skip reserved ranges

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

>>> Is not booting worse than what we have now -- which is, booting
>>> successfully but (probably) having issues due to MMIO ranges
>>> overlapping RMRRs?
>>
>> Its really so rare possibility here since in the real world we didn't
>> see any RMRR regions >= 0xF0000000 (the default pci memory start.) And I
>> already sent out a little better revision in that ensuing email so also
>> please take a review if possible :)
>
> Do remember the context we're talking about. :-)  Jan said, *if* there
> was a device that had an RMRR conflict with the "default" MMIO
> placement, then the guest simply wouldn't boot.  I was saying, in that

I understand what you guys mean. Yes, "BUG" is not good in our case :)

> case, we move from "silently ignore the conflict, possibly making the
> device not work" to "guest refuses to boot".  Which, if it was
> guaranteed that a conflict would cause the device to no longer work,
> would be an improvement.

This is really what I did this with "a little better revision" as I 
mentioned above. Maybe you're missing this, so I'd like to paste that 
below ( but if you already saw this please ignore this below )

     /* If pci bars conflict with RDM we need to disable this pci device. */
     for ( devfn = 0; devfn < 256; devfn++ )
     {
         bar_sz = pci_readl(devfn, bar_reg);
         bar_data = pci_readl(devfn, bar_reg);
         bar_data_upper = pci_readl(devfn, bar_reg + 4);
         /* Until here we don't conflict high memory. */
         if ( bar_data_upper )
             continue;

         for ( i = 0; i < memory_map.nr_map ; i++ )
         {
             uint64_t reserved_start, reserved_size;
             reserved_start = memory_map.map[i].addr;
             reserved_size = memory_map.map[i].size;
             if ( check_overlap(bar_data & ~(bar_sz - 1), bar_sz,
                                reserved_start, reserved_size) )
             {
                 printf("Reserved device memory conflicts with this pci 
bar,"
                        " so just disable this device.\n");
                 /* Now disable this device */
                 cmd = pci_readw(devfn, PCI_COMMAND);
                 pci_writew(devfn, PCI_COMMAND, ~cmd);
             }
         }
     }

Here I don't break that original allocation mechanism. Instead, I just 
check if we really have that conflict case and then disable that 
associated device.

>
>>> This patch series as a whole represents a lot of work and a lot of
>>> tangible improvements to the situation; and (unless the situation has
>>> changed) it's almost entirely acked apart from the MMIO placement
>>> part.  If there is a simple way that we can change hvmloader so that
>>> most (or even many) VM/device combinations work properly for the 4.6
>>> release, then I think it's worth considering.
>>>
>>
>> Current MMIO allocation mechanism is not good. So we really need to
>> reshape that, but we'd better do this with some further discussion in
>> next release :)
>
> Absolutely; I was saying, if we can put in a "good enough" measure for
> this release, then we can get the rest of the patch series in with our
> "good enough" hvmloader fix, and then work on fixing it properly next
> release.
>
> But if you're not aiming for this release anymore, then our aims are
> something different.  (See my other e-mail.)
>

I really still try to strive for 4.6 release if possible :)

Thanks
Tiejun