Subject: Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs

From: Scott Wood <[email-masked]>
Date: Wed, 6 Jun 2012 16:15:28 -0500

On 06/05/2012 10:50 PM, Ben Collins wrote:
> The commit introducing pcibios_io_space_offset() was ignoring 32-bit to
> 64-bit sign extention, which is the case on ppc32 with 64-bit resource
> addresses. This only seems to have shown up while running under QEMU for
> e500mc target. It may or may be suboptimal that QEMU has an IO base
> address > 32-bits for the e500-pci implementation, but 1) it's still a
> regression and 2) it's more correct to handle things this way.

Where do you see addresses over 32 bits in QEMU's e500-pci, at least
with current mainline QEMU and the mpc8544ds model?

I/O space should be at 0xe1000000.

I'm also not sure what this has to do with the virtual address returned
by ioremap().

> Signed-off-by: Ben Collins <[email-masked]>
> Cc: Benjamin Herrenschmidt <[email-masked]>
> ---
>  arch/powerpc/kernel/pci-common.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 8e78e93..be9ced7 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1477,9 +1477,15 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
>  	return pci_enable_resources(dev, mask);
>  }
>  
> +/* Before assuming too much here, take care to realize that we need sign
> + * extension from 32-bit pointers to 64-bit resource addresses to work.
> + */
>  resource_size_t pcibios_io_space_offset(struct pci_controller *hose)
>  {
> -	return (unsigned long) hose->io_base_virt - _IO_BASE;
> +	long vbase = (long)hose->io_base_virt;
> +	long io_base = _IO_BASE;
> +
> +	return (resource_size_t)(vbase - io_base);

Why do we want sign extension here?

If we do want it, there are a lot of other places in this file where the
same calculation is done.

-Scott