Re: [Qemu-ppc] [PATCH|RFC 1/1] pci: allow 0 address for PCI IO/MEM regions

[Laurent Vivier]

On 23/07/2015 09:15, Alexander Graf wrote:
> 
> 
>> Am 23.07.2015 um 06:01 schrieb David Gibson
>> <address@hidden>:
>> 
>>> On Wed, Jul 22, 2015 at 07:17:16PM +0200, Alexander Graf wrote:
>>>> On 07/22/15 19:08, Laurent Vivier wrote:
>>>> 
>>>>> On 22/07/2015 18:47, Alexander Graf wrote:
>>>>>> On 07/22/15 18:17, Michael S. Tsirkin wrote:
>>>>>>> On Wed, Jul 22, 2015 at 01:54:59PM +0200, Laurent Vivier
>>>>>>> wrote: From: Michael Roth <address@hidden>
>>>>>>> 
>>>>>>> Some kernels program a 0 address for io regions. PCI 3.0
>>>>>>> spec section 6.2.5.1 doesn't seem to disallow this.
>>>>>>> 
>>>>>>> Signed-off-by: Michael Roth <address@hidden> 
>>>>>>> [lvivier: add accept_addr_0 in PCIBus to conditionally
>>>>>>> allow addr 0, as this can break other architectures] 
>>>>>>> Signed-off-by: Laurent Vivier <address@hidden>
>>>>>> I guess it's a solution - though I'd rather fix up
>>>>>> priorities for PC and others.
>>>>> Won't this break Linux? IIRC there was a check in Linux that
>>>>> declares IO==0 addresses as invalid on PCI enumeration.
>>>> Well, without this patch, it doesn't work (try 2.4.0-rcX -M
>>>> pseries and add a virtio-net-pci card).
>>>> 
>>>> But the address 0 appears only on PCI hotplug. The first card
>>>> we add "lively" is always inserted with address 0 for BAR0
>>>> (even if we have already a PCI card on the bus).
>>>> 
>>>> If we add a second card, it works well (as it can't have the
>>>> address 0 again).
>>>> 
>>>> If we reboot the machine, the cards are reordered and have
>>>> "valid" (!= 0) addresses.
>>>> 
>>>> Do you suspect a bug in the kernel PCI hotplug functions ?
>>> 
>>> I'm not quite sure whether there is an actually visible bug, but
>>> I wanted to make sure you're aware that I've run into horrible
>>> issues on real hardware that exposed IO BARs at address 0 and
>>> then told Linux to not reconfigure BARs. Because then some code
>>> somewhere hidden deep inside Linux just assumes that the BAR is
>>> invalid.
>>> 
>>> IIRC BAR mapping for sPAPR hotplug happens in QEMU, no? So you
>>> can just guarantee that it never hits 0.
>> 
>> It should happen in qemu, but it doesn't, yet.
>> 
>> For boot-time devices, BAR allocation is done in SLOF, even though
>> bus enumeration is done in qemu, and qemu does have code to
>> publish assigned-addresses if anything had set them.
>> 
>> For hotplugged devices, the code is currently relying on some
>> weird behaviour in Linux guests that causes them to fall back to
>> assigning BARs themselves, even though they shouldn't under PAPR.
>> It's the guest itself attempting to assign address 0 and failing.
> 
> So Linux itself tries to assign a bar to address 0? That sounds
> incredibly odd. What code exactly does this? Is it generic or papr
> specific?

ok, I know what happens...

On hotplug, the address is checked by the function
pcibios_align_resource(), and for powerpc it is:

resource_size_t pcibios_align_resource(void *data, const struct resource
*res,
                                resource_size_t size, resource_size_t align)
{
        struct pci_dev *dev = data;
        resource_size_t start = res->start;

        if (res->flags & IORESOURCE_IO) {
                if (skip_isa_ioresource_align(dev))
                        return start;
                if (start & 0x300)
                        start = (start + 0x3ff) & ~0x3ff;
        }

        return start;
}

which has been copied from arch/x86/pci/i386.c (according to the comment
from other copies...).

So, yes, 0 address is valid...

Some architectures can define a PCIBIOS_MIN_IO, for instance on alpha:

...
                if (start - hose->io_space->start < PCIBIOS_MIN_IO)
                        start = PCIBIOS_MIN_IO + hose->io_space->start;
...

but I think this is because of physical constraint.

For powerpc, it seems valid PCI ranges are extracted from the OF
(pci_process_bridge_OF_ranges()), so from the point of view of the
hardware, BAR base address is always valid (and can be 0).

Put may be I've missed something...

Laurent