diff mbox series

[v2,4/8] hw/pci-host/bonito: Fixup pci.lomem mapping

Message ID 20201219071816.37963-2-jiaxun.yang@flygoat.com (mailing list archive)
State New, archived
Headers show
Series hm/mips/fuloong2e fixes | expand

Commit Message

Jiaxun Yang Dec. 19, 2020, 7:18 a.m. UTC
The original mapping had wrong base address.
Fix by correct the base adress and merge three alias into
a single.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 hw/pci-host/bonito.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

Comments

Philippe Mathieu-Daudé Dec. 21, 2020, 8:45 p.m. UTC | #1
On 12/19/20 8:18 AM, Jiaxun Yang wrote:
> The original mapping had wrong base address.
> Fix by correct the base adress and merge three alias into
> a single.

Why merge? Beside, typo "address".

> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>  hw/pci-host/bonito.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index 43b79448a9..3a31ba42f2 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -608,7 +608,7 @@ static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
>  {
>      PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>      BonitoState *bs = BONITO_PCI_HOST_BRIDGE(dev);
> -    MemoryRegion *pcimem_lo_alias = g_new(MemoryRegion, 3);
> +    MemoryRegion *pcimem_lo_alias = g_new(MemoryRegion, 1);
>  
>      memory_region_init(&bs->pci_mem, OBJECT(dev), "pci.mem", BONITO_PCIHI_SIZE);
>      phb->bus = pci_register_root_bus(dev, "pci",
> @@ -616,16 +616,11 @@ static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
>                                       dev, &bs->pci_mem, get_system_io(),
>                                       0x28, 32, TYPE_PCI_BUS);
>  
> -    for (size_t i = 0; i < 3; i++) {
> -        char *name = g_strdup_printf("pci.lomem%zu", i);
> -
> -        memory_region_init_alias(&pcimem_lo_alias[i], NULL, name,
> -                                 &bs->pci_mem, i * 64 * MiB, 64 * MiB);
> -        memory_region_add_subregion(get_system_memory(),
> -                                    BONITO_PCILO_BASE + i * 64 * MiB,
> -                                    &pcimem_lo_alias[i]);
> -        g_free(name);
> -    }
> +    memory_region_init_alias(pcimem_lo_alias, OBJECT(dev), "pci.lomem",
> +                             &bs->pci_mem, BONITO_PCILO_BASE,
> +                             BONITO_PCILO_SIZE);

Why is your pci_mem mapped at 0?

> +    memory_region_add_subregion(get_system_memory(), BONITO_PCILO_BASE,
> +                                pcimem_lo_alias);
>  
>      create_unimplemented_device("pci.io", BONITO_PCIIO_BASE, 1 * MiB);
>  }
>
Jiaxun Yang Dec. 22, 2020, 12:36 a.m. UTC | #2
在 2020/12/22 上午4:45, Philippe Mathieu-Daudé 写道:
> On 12/19/20 8:18 AM, Jiaxun Yang wrote:
>> The original mapping had wrong base address.
>> Fix by correct the base adress and merge three alias into
>> a single.
> Why merge? Beside, typo "address".


Hi Philippe,

Thanks for your reviewing!

Because I can't understand why it was in three pieces.
I was just trying to do what kernel as I don't have much knowledge with
Fuloong2E.

The kernel treated PCI region as a whole part[1] at 0x10000000 with size
0x0c000000.

It fixed long lasting radeonfb starting failure.

>
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>   hw/pci-host/bonito.c | 17 ++++++-----------
>>   1 file changed, 6 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
>> index 43b79448a9..3a31ba42f2 100644
>> --- a/hw/pci-host/bonito.c
>> +++ b/hw/pci-host/bonito.c
>> @@ -608,7 +608,7 @@ static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
>>   {
>>       PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>>       BonitoState *bs = BONITO_PCI_HOST_BRIDGE(dev);
>> -    MemoryRegion *pcimem_lo_alias = g_new(MemoryRegion, 3);
>> +    MemoryRegion *pcimem_lo_alias = g_new(MemoryRegion, 1);
>>   
>>       memory_region_init(&bs->pci_mem, OBJECT(dev), "pci.mem", BONITO_PCIHI_SIZE);
>>       phb->bus = pci_register_root_bus(dev, "pci",
>> @@ -616,16 +616,11 @@ static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
>>                                        dev, &bs->pci_mem, get_system_io(),
>>                                        0x28, 32, TYPE_PCI_BUS);
>>   
>> -    for (size_t i = 0; i < 3; i++) {
>> -        char *name = g_strdup_printf("pci.lomem%zu", i);
>> -
>> -        memory_region_init_alias(&pcimem_lo_alias[i], NULL, name,
>> -                                 &bs->pci_mem, i * 64 * MiB, 64 * MiB);
>> -        memory_region_add_subregion(get_system_memory(),
>> -                                    BONITO_PCILO_BASE + i * 64 * MiB,
>> -                                    &pcimem_lo_alias[i]);
>> -        g_free(name);
>> -    }
>> +    memory_region_init_alias(pcimem_lo_alias, OBJECT(dev), "pci.lomem",
>> +                             &bs->pci_mem, BONITO_PCILO_BASE,
>> +                             BONITO_PCILO_SIZE);
> Why is your pci_mem mapped at 0?

It is actually started at 0x10000000.

As: #define BONITO_PCILO_BASE       0x10000000


Thanks.

[1]: 
https://elixir.bootlin.com/linux/latest/source/arch/mips/loongson2ef/common/pci.c

- Jiaxun
>
>> +    memory_region_add_subregion(get_system_memory(), BONITO_PCILO_BASE,
>> +                                pcimem_lo_alias);
>>   
>>       create_unimplemented_device("pci.io", BONITO_PCIIO_BASE, 1 * MiB);
>>   }
>>
Philippe Mathieu-Daudé Dec. 22, 2020, 12:13 p.m. UTC | #3
On 12/22/20 1:36 AM, Jiaxun Yang wrote:
> 在 2020/12/22 上午4:45, Philippe Mathieu-Daudé 写道:
>> On 12/19/20 8:18 AM, Jiaxun Yang wrote:
>>> The original mapping had wrong base address.
>>> Fix by correct the base adress and merge three alias into
>>> a single.
>> Why merge? Beside, typo "address".
> 
> 
> Hi Philippe,
> 
> Thanks for your reviewing!
> 
> Because I can't understand why it was in three pieces.
> I was just trying to do what kernel as I don't have much knowledge with
> Fuloong2E.

You can find the information in the 'BONITO64 - "north bridge"
controller for 64-bit MIPS CPUs" datasheet from Algorithmics Ltd. 2001,
section "4.1. Address maps" and Table 4.1: "CPU/local bus address map".

See also "Design and Implementation of EBoot based on Godson Platform":
http://www.ecice06.com/CN/article/downloadArticleFile.do?attachType=PDF&id=10656

> 
> The kernel treated PCI region as a whole part[1] at 0x10000000 with size
> 0x0c000000.
> 
> It fixed long lasting radeonfb starting failure.
> 
>>
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> ---
>>>   hw/pci-host/bonito.c | 17 ++++++-----------
>>>   1 file changed, 6 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
>>> index 43b79448a9..3a31ba42f2 100644
>>> --- a/hw/pci-host/bonito.c
>>> +++ b/hw/pci-host/bonito.c
>>> @@ -608,7 +608,7 @@ static void bonito_pcihost_realize(DeviceState
>>> *dev, Error **errp)
>>>   {
>>>       PCIHostState *phb = PCI_HOST_BRIDGE(dev);
>>>       BonitoState *bs = BONITO_PCI_HOST_BRIDGE(dev);
>>> -    MemoryRegion *pcimem_lo_alias = g_new(MemoryRegion, 3);
>>> +    MemoryRegion *pcimem_lo_alias = g_new(MemoryRegion, 1);
>>>         memory_region_init(&bs->pci_mem, OBJECT(dev), "pci.mem",
>>> BONITO_PCIHI_SIZE);
>>>       phb->bus = pci_register_root_bus(dev, "pci",
>>> @@ -616,16 +616,11 @@ static void bonito_pcihost_realize(DeviceState
>>> *dev, Error **errp)
>>>                                        dev, &bs->pci_mem,
>>> get_system_io(),
>>>                                        0x28, 32, TYPE_PCI_BUS);
>>>   -    for (size_t i = 0; i < 3; i++) {
>>> -        char *name = g_strdup_printf("pci.lomem%zu", i);
>>> -
>>> -        memory_region_init_alias(&pcimem_lo_alias[i], NULL, name,
>>> -                                 &bs->pci_mem, i * 64 * MiB, 64 * MiB);
>>> -        memory_region_add_subregion(get_system_memory(),
>>> -                                    BONITO_PCILO_BASE + i * 64 * MiB,
>>> -                                    &pcimem_lo_alias[i]);
>>> -        g_free(name);
>>> -    }
>>> +    memory_region_init_alias(pcimem_lo_alias, OBJECT(dev), "pci.lomem",
>>> +                             &bs->pci_mem, BONITO_PCILO_BASE,
>>> +                             BONITO_PCILO_SIZE);
>> Why is your pci_mem mapped at 0?
> 
> It is actually started at 0x10000000.
> 
> As: #define BONITO_PCILO_BASE       0x10000000
> 
> 
> Thanks.
> 
> [1]:
> https://elixir.bootlin.com/linux/latest/source/arch/mips/loongson2ef/common/pci.c
> 
> 
> - Jiaxun
>>
>>> +    memory_region_add_subregion(get_system_memory(), BONITO_PCILO_BASE,
>>> +                                pcimem_lo_alias);
>>>         create_unimplemented_device("pci.io", BONITO_PCIIO_BASE, 1 *
>>> MiB);
>>>   }
>>>
> 
>
diff mbox series

Patch

diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index 43b79448a9..3a31ba42f2 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -608,7 +608,7 @@  static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
 {
     PCIHostState *phb = PCI_HOST_BRIDGE(dev);
     BonitoState *bs = BONITO_PCI_HOST_BRIDGE(dev);
-    MemoryRegion *pcimem_lo_alias = g_new(MemoryRegion, 3);
+    MemoryRegion *pcimem_lo_alias = g_new(MemoryRegion, 1);
 
     memory_region_init(&bs->pci_mem, OBJECT(dev), "pci.mem", BONITO_PCIHI_SIZE);
     phb->bus = pci_register_root_bus(dev, "pci",
@@ -616,16 +616,11 @@  static void bonito_pcihost_realize(DeviceState *dev, Error **errp)
                                      dev, &bs->pci_mem, get_system_io(),
                                      0x28, 32, TYPE_PCI_BUS);
 
-    for (size_t i = 0; i < 3; i++) {
-        char *name = g_strdup_printf("pci.lomem%zu", i);
-
-        memory_region_init_alias(&pcimem_lo_alias[i], NULL, name,
-                                 &bs->pci_mem, i * 64 * MiB, 64 * MiB);
-        memory_region_add_subregion(get_system_memory(),
-                                    BONITO_PCILO_BASE + i * 64 * MiB,
-                                    &pcimem_lo_alias[i]);
-        g_free(name);
-    }
+    memory_region_init_alias(pcimem_lo_alias, OBJECT(dev), "pci.lomem",
+                             &bs->pci_mem, BONITO_PCILO_BASE,
+                             BONITO_PCILO_SIZE);
+    memory_region_add_subregion(get_system_memory(), BONITO_PCILO_BASE,
+                                pcimem_lo_alias);
 
     create_unimplemented_device("pci.io", BONITO_PCIIO_BASE, 1 * MiB);
 }