| Message ID | 20201219071816.37963-2-jiaxun.yang@flygoat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hm/mips/fuloong2e fixes | expand |
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); > } >
在 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); >> } >>
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 --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); }
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(-)