[Qemu-devel] [RFC][PATCH] PCI: fix pci_to_cpu_addr() issue

[Qemu-devel] [RFC][PATCH] PCI: fix pci_to_cpu_addr() issue

[Huacai Chen]

It seems like software may both use CPU address or PCI address to access a PCI
device. For example, Bonito north bridge map PCI memory space at 0x10000000 ~ 
0x1C000000. PMON code use 0x00000000 ~ 0x0C000000, but Linux kernel code use 
0x10000000 ~ 0x1C000000 to access devices. If set pci_mem_base to 0, PMON can't
work, but if set pci_mem_base to 0x10000000, Linux can't access PCI. So I make
this patch to make both cases works.

However, I don't know whether the modification will break other archs, so
request for comments here.

Signed-off-by: Huacai Chen <zltjiangshi@gmail.com>
---
 hw/pci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 7787005..50e3572 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -672,7 +672,7 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
 static target_phys_addr_t pci_to_cpu_addr(PCIBus *bus,
                                           target_phys_addr_t addr)
 {
-    return addr + bus->mem_base;
+    return addr | bus->mem_base;
 }
 
 static void pci_unregister_io_regions(PCIDevice *pci_dev)
-- 
1.7.0.4

[Qemu-devel] Re: [RFC][PATCH] PCI: fix pci_to_cpu_addr() issue

[Isaku Yamahata]

Can you elaborate on how pci bus is mapped into local bus?
Is there specification publicly available? Google didn't tell me.


On Wed, Jun 30, 2010 at 06:39:53PM +0800, Huacai Chen wrote:
> It seems like software may both use CPU address or PCI address to access a PCI
> device. For example, Bonito north bridge map PCI memory space at 0x10000000 ~ 
> 0x1C000000. PMON code use 0x00000000 ~ 0x0C000000, but Linux kernel code use 
> 0x10000000 ~ 0x1C000000 to access devices. If set pci_mem_base to 0, PMON can't
> work, but if set pci_mem_base to 0x10000000, Linux can't access PCI. So I make
> this patch to make both cases works.
> 
> However, I don't know whether the modification will break other archs, so
> request for comments here.
> 
> Signed-off-by: Huacai Chen <zltjiangshi@gmail.com>
> ---
>  hw/pci.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 7787005..50e3572 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -672,7 +672,7 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
>  static target_phys_addr_t pci_to_cpu_addr(PCIBus *bus,
>                                            target_phys_addr_t addr)
>  {
> -    return addr + bus->mem_base;
> +    return addr | bus->mem_base;
>  }
>  
>  static void pci_unregister_io_regions(PCIDevice *pci_dev)
> -- 
> 1.7.0.4
> 

-- 
yamahata

[Qemu-devel] Re: [RFC][PATCH] PCI: fix pci_to_cpu_addr() issue

[chen huacai]

Maybe this is what you want, please look at Page 10.
http://people.openrays.org/~comcat/godson/doc/godson2e.north.bridge.manual.pdf
But it is written in Chinese, I'm sorry that I also don't have an
English version.


On Wed, Jun 30, 2010 at 9:38 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> Can you elaborate on how pci bus is mapped into local bus?
> Is there specification publicly available? Google didn't tell me.
>
>
> On Wed, Jun 30, 2010 at 06:39:53PM +0800, Huacai Chen wrote:
>> It seems like software may both use CPU address or PCI address to access a PCI
>> device. For example, Bonito north bridge map PCI memory space at 0x10000000 ~
>> 0x1C000000. PMON code use 0x00000000 ~ 0x0C000000, but Linux kernel code use
>> 0x10000000 ~ 0x1C000000 to access devices. If set pci_mem_base to 0, PMON can't
>> work, but if set pci_mem_base to 0x10000000, Linux can't access PCI. So I make
>> this patch to make both cases works.
>>
>> However, I don't know whether the modification will break other archs, so
>> request for comments here.
>>
>> Signed-off-by: Huacai Chen <zltjiangshi@gmail.com>
>> ---
>>  hw/pci.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/hw/pci.c b/hw/pci.c
>> index 7787005..50e3572 100644
>> --- a/hw/pci.c
>> +++ b/hw/pci.c
>> @@ -672,7 +672,7 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
>>  static target_phys_addr_t pci_to_cpu_addr(PCIBus *bus,
>>                                            target_phys_addr_t addr)
>>  {
>> -    return addr + bus->mem_base;
>> +    return addr | bus->mem_base;
>>  }
>>
>>  static void pci_unregister_io_regions(PCIDevice *pci_dev)
>> --
>> 1.7.0.4
>>
>
> --
> yamahata
>



-- 
Huacai Chen

Re: [Qemu-devel] Re: [RFC][PATCH] PCI: fix pci_to_cpu_addr() issue

[Isaku Yamahata]

Thank you the pointer. I found the followings.
https://groups.google.com/group/archlinux-for-loongson/web/loongson
https://groups.google.com/group/archlinux-for-loongson/web/bonito64-spec.pdf
Am I referring to a correct spec?

According to it,
[0x00000000,  0x10000000) RAM
[0x10000000,  0x14000000) PCI_Lo0
[0x14000000,  0x18000000) PCI_Lo1
[0x18000000,  0x1c000000) PCI_Lo2
[0x20000000,  0x80000000) PCI_1.5
[0x80000000, 0x100000000) PCI_2		

[0x00000000, 0x0c000000) in physical address is RAM, so I don't understand PMON uses
the area. I may be misunderstanding something.
Can you elaborate please?

PCI_Lo[123] is interesting. The base address can be programmed independently.
Such operation isn't assumed by qemu.


On Wed, Jun 30, 2010 at 10:05:55PM +0800, chen huacai wrote:
> Maybe this is what you want, please look at Page 10.
> http://people.openrays.org/~comcat/godson/doc/godson2e.north.bridge.manual.pdf
> But it is written in Chinese, I'm sorry that I also don't have an
> English version.
> 
> 
> On Wed, Jun 30, 2010 at 9:38 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> > Can you elaborate on how pci bus is mapped into local bus?
> > Is there specification publicly available? Google didn't tell me.
> >
> >
> > On Wed, Jun 30, 2010 at 06:39:53PM +0800, Huacai Chen wrote:
> >> It seems like software may both use CPU address or PCI address to access a PCI
> >> device. For example, Bonito north bridge map PCI memory space at 0x10000000 ~
> >> 0x1C000000. PMON code use 0x00000000 ~ 0x0C000000, but Linux kernel code use
> >> 0x10000000 ~ 0x1C000000 to access devices. If set pci_mem_base to 0, PMON can't
> >> work, but if set pci_mem_base to 0x10000000, Linux can't access PCI. So I make
> >> this patch to make both cases works.
> >>
> >> However, I don't know whether the modification will break other archs, so
> >> request for comments here.
> >>
> >> Signed-off-by: Huacai Chen <zltjiangshi@gmail.com>
> >> ---
> >> ??hw/pci.c | ?? ??2 +-
> >> ??1 files changed, 1 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/hw/pci.c b/hw/pci.c
> >> index 7787005..50e3572 100644
> >> --- a/hw/pci.c
> >> +++ b/hw/pci.c
> >> @@ -672,7 +672,7 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
> >> ??static target_phys_addr_t pci_to_cpu_addr(PCIBus *bus,
> >> ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??target_phys_addr_t addr)
> >> ??{
> >> - ?? ??return addr + bus->mem_base;
> >> + ?? ??return addr | bus->mem_base;
> >> ??}
> >>
> >> ??static void pci_unregister_io_regions(PCIDevice *pci_dev)
> >> --
> >> 1.7.0.4
> >>
> >
> > --
> > yamahata
> >
> 
> 
> 
> -- 
> Huacai Chen
> 

-- 
yamahata

Re: [Qemu-devel] Re: [RFC][PATCH] PCI: fix pci_to_cpu_addr() issue

[chen huacai]

I have some doubts: when newcfg=0, Qemu Monitor shows
BAR6: 32bit memory at 0x04000000 [0x0400ffff]
Does this means the physical address 0x04000000 isn't in RAM but in PCI memory?
If yes, seems like it will cause problems.
If no, how to understand the output of "info pci" in Qemu Monitor?

On Fri, Jul 2, 2010 at 6:13 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> On Fri, Jul 02, 2010 at 03:44:05PM +0800, chen huacai wrote:
>> Maybe I made some mistakes, or maybe PMON has bugs. :)
>>
>> Please see the PMON code online at
>> http://www.loongson.cn/svn/pmon-loongson/trunk/Targets/Bonito2edev/pci/pci_machdep.c
>> In function _pci_hwinit(), if newcfg=0, PMON will use [0x00000000,
>> 0x0c000000) to fill BONITO_PCIMAP register.
>
> Oh I see. You're talking about pci bus address.
>
>
>> When boot to PMON (newcfg=0), I'll see something like this in Qemu Monitor
>> BAR6: 32bit memory at 0x04000000 [0x0400ffff]
>> Then, after linux kernel loaded, Qemu Monitor shows:
>> BAR6: 32bit memory at 0x14000000 [0x1400ffff]
>
> Do you mean that Linux kernel reprograms BONITO_PCIMAP?
> I.e.
> PMON maps [0x04000000, 0x0c000000) as Lo[123]in pci bus address with newcfg = 0,
>          [0x00000000, 0x04000000) as Lo0, [0x14000000, 0x1c000000) as Lo[12] with newcfg = 1.
> Linux maps [0x10000000, 0x1c000000) as Lo[123].
>
> Then what you want is remapping, not changing pci_to_cpu_addr().
> When pcimap register is modified, update the mapping.
> Probably pci_bridge_update_mappings() would help,
>
> PMON's non-contiguous mapping (newcfg = 1) is challenging,
> newcfg = 0 case would be easier.
>
>
>>
>> If rebuild PMON with newcfg=1, Qemu Monitor show the same info before
>> and after kernel loaded:
>> BAR6: 32bit memory at 0x14000000 [0x1400ffff]
>>
>> On Fri, Jul 2, 2010 at 10:11 AM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
>> > Thank you the pointer. I found the followings.
>> > https://groups.google.com/group/archlinux-for-loongson/web/loongson
>> > https://groups.google.com/group/archlinux-for-loongson/web/bonito64-spec.pdf
>> > Am I referring to a correct spec?
>> >
>> > According to it,
>> > [0x00000000, ??0x10000000) RAM
>> > [0x10000000, ??0x14000000) PCI_Lo0
>> > [0x14000000, ??0x18000000) PCI_Lo1
>> > [0x18000000, ??0x1c000000) PCI_Lo2
>> > [0x20000000, ??0x80000000) PCI_1.5
>> > [0x80000000, 0x100000000) PCI_2
>> >
>> > [0x00000000, 0x0c000000) in physical address is RAM, so I don't understand PMON uses
>> > the area. I may be misunderstanding something.
>> > Can you elaborate please?
>> >
>> > PCI_Lo[123] is interesting. The base address can be programmed independently.
>> > Such operation isn't assumed by qemu.
>> >
>> >
>> > On Wed, Jun 30, 2010 at 10:05:55PM +0800, chen huacai wrote:
>> >> Maybe this is what you want, please look at Page 10.
>> >> http://people.openrays.org/~comcat/godson/doc/godson2e.north.bridge.manual.pdf
>> >> But it is written in Chinese, I'm sorry that I also don't have an
>> >> English version.
>> >>
>> >>
>> >> On Wed, Jun 30, 2010 at 9:38 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
>> >> > Can you elaborate on how pci bus is mapped into local bus?
>> >> > Is there specification publicly available? Google didn't tell me.
>> >> >
>> >> >
>> >> > On Wed, Jun 30, 2010 at 06:39:53PM +0800, Huacai Chen wrote:
>> >> >> It seems like software may both use CPU address or PCI address to access a PCI
>> >> >> device. For example, Bonito north bridge map PCI memory space at 0x10000000 ~
>> >> >> 0x1C000000. PMON code use 0x00000000 ~ 0x0C000000, but Linux kernel code use
>> >> >> 0x10000000 ~ 0x1C000000 to access devices. If set pci_mem_base to 0, PMON can't
>> >> >> work, but if set pci_mem_base to 0x10000000, Linux can't access PCI. So I make
>> >> >> this patch to make both cases works.
>> >> >>
>> >> >> However, I don't know whether the modification will break other archs, so
>> >> >> request for comments here.
>> >> >>
>> >> >> Signed-off-by: Huacai Chen <zltjiangshi@gmail.com>
>> >> >> ---
>> >> >> ??hw/pci.c | ?? ??2 +-
>> >> >> ??1 files changed, 1 insertions(+), 1 deletions(-)
>> >> >>
>> >> >> diff --git a/hw/pci.c b/hw/pci.c
>> >> >> index 7787005..50e3572 100644
>> >> >> --- a/hw/pci.c
>> >> >> +++ b/hw/pci.c
>> >> >> @@ -672,7 +672,7 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
>> >> >> ??static target_phys_addr_t pci_to_cpu_addr(PCIBus *bus,
>> >> >> ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??target_phys_addr_t addr)
>> >> >> ??{
>> >> >> - ?? ??return addr + bus->mem_base;
>> >> >> + ?? ??return addr | bus->mem_base;
>> >> >> ??}
>> >> >>
>> >> >> ??static void pci_unregister_io_regions(PCIDevice *pci_dev)
>> >> >> --
>> >> >> 1.7.0.4
>> >> >>
>> >> >
>> >> > --
>> >> > yamahata
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Huacai Chen
>> >>
>> >
>> > --
>> > yamahata
>> >
>>
>>
>>
>> --
>> Huacai Chen
>>
>
> --
> yamahata
>



-- 
Huacai Chen

Re: [Qemu-devel] Re: [RFC][PATCH] PCI: fix pci_to_cpu_addr() issue

[Isaku Yamahata]

You're confusing pci bus address with physical address.
BAR is pci bus address.

On Sat, Jul 03, 2010 at 12:11:58PM +0800, chen huacai wrote:
> I have some doubts: when newcfg=0, Qemu Monitor shows
> BAR6: 32bit memory at 0x04000000 [0x0400ffff]
> Does this means the physical address 0x04000000 isn't in RAM but in PCI memory?
> If yes, seems like it will cause problems.
> If no, how to understand the output of "info pci" in Qemu Monitor?
> 
> On Fri, Jul 2, 2010 at 6:13 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> > On Fri, Jul 02, 2010 at 03:44:05PM +0800, chen huacai wrote:
> >> Maybe I made some mistakes, or maybe PMON has bugs. :)
> >>
> >> Please see the PMON code online at
> >> http://www.loongson.cn/svn/pmon-loongson/trunk/Targets/Bonito2edev/pci/pci_machdep.c
> >> In function _pci_hwinit(), if newcfg=0, PMON will use [0x00000000,
> >> 0x0c000000) to fill BONITO_PCIMAP register.
> >
> > Oh I see. You're talking about pci bus address.
> >
> >
> >> When boot to PMON (newcfg=0), I'll see something like this in Qemu Monitor
> >> BAR6: 32bit memory at 0x04000000 [0x0400ffff]
> >> Then, after linux kernel loaded, Qemu Monitor shows:
> >> BAR6: 32bit memory at 0x14000000 [0x1400ffff]
> >
> > Do you mean that Linux kernel reprograms BONITO_PCIMAP?
> > I.e.
> > PMON maps [0x04000000, 0x0c000000) as Lo[123]in pci bus address with newcfg = 0,
> > ?? ?? ?? ?? ??[0x00000000, 0x04000000) as Lo0, [0x14000000, 0x1c000000) as Lo[12] with newcfg = 1.
> > Linux maps [0x10000000, 0x1c000000) as Lo[123].
> >
> > Then what you want is remapping, not changing pci_to_cpu_addr().
> > When pcimap register is modified, update the mapping.
> > Probably pci_bridge_update_mappings() would help,
> >
> > PMON's non-contiguous mapping (newcfg = 1) is challenging,
> > newcfg = 0 case would be easier.
> >
> >
> >>
> >> If rebuild PMON with newcfg=1, Qemu Monitor show the same info before
> >> and after kernel loaded:
> >> BAR6: 32bit memory at 0x14000000 [0x1400ffff]
> >>
> >> On Fri, Jul 2, 2010 at 10:11 AM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> >> > Thank you the pointer. I found the followings.
> >> > https://groups.google.com/group/archlinux-for-loongson/web/loongson
> >> > https://groups.google.com/group/archlinux-for-loongson/web/bonito64-spec.pdf
> >> > Am I referring to a correct spec?
> >> >
> >> > According to it,
> >> > [0x00000000, ??0x10000000) RAM
> >> > [0x10000000, ??0x14000000) PCI_Lo0
> >> > [0x14000000, ??0x18000000) PCI_Lo1
> >> > [0x18000000, ??0x1c000000) PCI_Lo2
> >> > [0x20000000, ??0x80000000) PCI_1.5
> >> > [0x80000000, 0x100000000) PCI_2
> >> >
> >> > [0x00000000, 0x0c000000) in physical address is RAM, so I don't understand PMON uses
> >> > the area. I may be misunderstanding something.
> >> > Can you elaborate please?
> >> >
> >> > PCI_Lo[123] is interesting. The base address can be programmed independently.
> >> > Such operation isn't assumed by qemu.
> >> >
> >> >
> >> > On Wed, Jun 30, 2010 at 10:05:55PM +0800, chen huacai wrote:
> >> >> Maybe this is what you want, please look at Page 10.
> >> >> http://people.openrays.org/~comcat/godson/doc/godson2e.north.bridge.manual.pdf
> >> >> But it is written in Chinese, I'm sorry that I also don't have an
> >> >> English version.
> >> >>
> >> >>
> >> >> On Wed, Jun 30, 2010 at 9:38 PM, Isaku Yamahata <yamahata@valinux.co.jp> wrote:
> >> >> > Can you elaborate on how pci bus is mapped into local bus?
> >> >> > Is there specification publicly available? Google didn't tell me.
> >> >> >
> >> >> >
> >> >> > On Wed, Jun 30, 2010 at 06:39:53PM +0800, Huacai Chen wrote:
> >> >> >> It seems like software may both use CPU address or PCI address to access a PCI
> >> >> >> device. For example, Bonito north bridge map PCI memory space at 0x10000000 ~
> >> >> >> 0x1C000000. PMON code use 0x00000000 ~ 0x0C000000, but Linux kernel code use
> >> >> >> 0x10000000 ~ 0x1C000000 to access devices. If set pci_mem_base to 0, PMON can't
> >> >> >> work, but if set pci_mem_base to 0x10000000, Linux can't access PCI. So I make
> >> >> >> this patch to make both cases works.
> >> >> >>
> >> >> >> However, I don't know whether the modification will break other archs, so
> >> >> >> request for comments here.
> >> >> >>
> >> >> >> Signed-off-by: Huacai Chen <zltjiangshi@gmail.com>
> >> >> >> ---
> >> >> >> ??hw/pci.c | ?? ??2 +-
> >> >> >> ??1 files changed, 1 insertions(+), 1 deletions(-)
> >> >> >>
> >> >> >> diff --git a/hw/pci.c b/hw/pci.c
> >> >> >> index 7787005..50e3572 100644
> >> >> >> --- a/hw/pci.c
> >> >> >> +++ b/hw/pci.c
> >> >> >> @@ -672,7 +672,7 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
> >> >> >> ??static target_phys_addr_t pci_to_cpu_addr(PCIBus *bus,
> >> >> >> ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ?? ??target_phys_addr_t addr)
> >> >> >> ??{
> >> >> >> - ?? ??return addr + bus->mem_base;
> >> >> >> + ?? ??return addr | bus->mem_base;
> >> >> >> ??}
> >> >> >>
> >> >> >> ??static void pci_unregister_io_regions(PCIDevice *pci_dev)
> >> >> >> --
> >> >> >> 1.7.0.4
> >> >> >>
> >> >> >
> >> >> > --
> >> >> > yamahata
> >> >> >
> >> >>
> >> >>
> >> >>
> >> >> --
> >> >> Huacai Chen
> >> >>
> >> >
> >> > --
> >> > yamahata
> >> >
> >>
> >>
> >>
> >> --
> >> Huacai Chen
> >>
> >
> > --
> > yamahata
> >
> 
> 
> 
> -- 
> Huacai Chen
> 

-- 
yamahata