Re: [PATCH-for-9.1 v4 1/2] hw/pci-host/gt64120: Set PCI base address register write mask

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 2/8/24 21:21, BALATON Zoltan wrote:
> On Fri, 2 Aug 2024, Philippe Mathieu-Daudé wrote:
>> When booting Linux we see:
>>
>>  PCI host bridge to bus 0000:00
>>  pci_bus 0000:00: root bus resource [mem 0x10000000-0x17ffffff]
>>  pci_bus 0000:00: root bus resource [io  0x1000-0x1fffff]
>>  pci_bus 0000:00: No busn resource found for root bus, will use [bus 
>> 00-ff]
>>  pci 0000:00:00.0: [11ab:4620] type 00 class 0x060000
>>  pci 0000:00:00.0: [Firmware Bug]: reg 0x14: invalid BAR (can't size)
>>  pci 0000:00:00.0: [Firmware Bug]: reg 0x18: invalid BAR (can't size)
>>  pci 0000:00:00.0: [Firmware Bug]: reg 0x1c: invalid BAR (can't size)
>>  pci 0000:00:00.0: [Firmware Bug]: reg 0x20: invalid BAR (can't size)
>>  pci 0000:00:00.0: [Firmware Bug]: reg 0x24: invalid BAR (can't size)
>>
>> This is due to missing base address register write mask.
>> Add it to get:
>>
>>  PCI host bridge to bus 0000:00
>>  pci_bus 0000:00: root bus resource [mem 0x10000000-0x17ffffff]
>>  pci_bus 0000:00: root bus resource [io  0x1000-0x1fffff]
>>  pci_bus 0000:00: No busn resource found for root bus, will use [bus 
>> 00-ff]
>>  pci 0000:00:00.0: [11ab:4620] type 00 class 0x060000
>>  pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x00000fff pref]
>>  pci 0000:00:00.0: reg 0x14: [mem 0x01000000-0x01000fff pref]
>>  pci 0000:00:00.0: reg 0x18: [mem 0x1c000000-0x1c000fff]
>>  pci 0000:00:00.0: reg 0x1c: [mem 0x1f000000-0x1f000fff]
>>  pci 0000:00:00.0: reg 0x20: [mem 0x1be00000-0x1be00fff]
>>  pci 0000:00:00.0: reg 0x24: [io  0x14000000-0x14000fff]
>>
>> Since this device is only used by MIPS machines which aren't
>> versioned, we don't need to update migration compat machinery.
>>
>> Mention the datasheet referenced. Remove the "Malta assumptions
>> ahead" comment since the reset values from the datasheet are used.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>> hw/pci-host/gt64120.c | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)


>> static void gt64120_pci_realize(PCIDevice *d, Error **errp)
>> {
>> -    /* FIXME: Malta specific hw assumptions ahead */
>> +    /* Values from chapter 17.16 "PCI Configuration" */
>> +
>>     pci_set_word(d->config + PCI_COMMAND, 0);
>>     pci_set_word(d->config + PCI_STATUS,
>>                  PCI_STATUS_FAST_BACK | PCI_STATUS_DEVSEL_MEDIUM);
>>     pci_config_set_prog_interface(d->config, 0);
>> +
>> +    pci_set_long(d->wmask + PCI_BASE_ADDRESS_0, 0xfffff009);
>> +    pci_set_long(d->wmask + PCI_BASE_ADDRESS_1, 0xfffff009);
>> +    pci_set_long(d->wmask + PCI_BASE_ADDRESS_2, 0xfffff009);
>> +    pci_set_long(d->wmask + PCI_BASE_ADDRESS_3, 0xfffff009);
>> +    pci_set_long(d->wmask + PCI_BASE_ADDRESS_4, 0xfffff009);
> 
> Documentation says bit 0 is read only 0 for these? Why mask ending with 
> 9 not 8? Also prefetch bit 3 is read only 0 for the last one BAR4.

Oops indeed, and also Type/Prefetch for BAR5; updated as:

pci_set_long(d->wmask + PCI_BASE_ADDRESS_0, 0xfffff008); /* SCS[1:0] */
pci_set_long(d->wmask + PCI_BASE_ADDRESS_1, 0xfffff008); /* SCS[3:2] */
pci_set_long(d->wmask + PCI_BASE_ADDRESS_2, 0xfffff008); /* CS[2:0] */
pci_set_long(d->wmask + PCI_BASE_ADDRESS_3, 0xfffff008); /* CS[3], BootCS */
pci_set_long(d->wmask + PCI_BASE_ADDRESS_4, 0xfffff000); /* ISD MMIO */
pci_set_long(d->wmask + PCI_BASE_ADDRESS_5, 0xfffff001); /* ISD I/O */

> Otherwise:
> 
> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>

Thanks!

Phil.