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

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

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


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