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

From: Jiaxun Yang <jiaxun.yang@flygoat.com>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>, qemu-devel@nongnu.org
Cc: chenhuacai@kernel.org, wainersm@redhat.com, crosa@redhat.com
Subject: Re: [PATCH v2 4/8] hw/pci-host/bonito: Fixup pci.lomem mapping
Date: Tue, 22 Dec 2020 08:36:04 +0800	[thread overview]
Message-ID: <1089c6a2-e446-9312-4edb-090a6ae8bf72@flygoat.com> (raw)
In-Reply-To: <05d6819c-67dd-9275-e9df-1a26baf807ee@amsat.org>

在 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);
>>   }
>>