Re: [PULL 30/43] vt82c686: Fix SMBus IO base and configuration registers

[Philippe Mathieu-Daudé]

On 6/24/21 7:00 PM, BALATON Zoltan wrote:
> On Thu, 24 Jun 2021, Philippe Mathieu-Daudé wrote:
>> On 6/24/21 6:16 PM, Philippe Mathieu-Daudé wrote:
>>> On 6/24/21 6:01 PM, Philippe Mathieu-Daudé wrote:
>>>> On 6/24/21 5:46 PM, Philippe Mathieu-Daudé wrote:
>>>>> Hi Zoltan,
>>>>>
>>>>> On 2/21/21 3:34 PM, Philippe Mathieu-Daudé wrote:
>>>>>> From: BALATON Zoltan <balaton@eik.bme.hu>
>>>>>>
>>>>>> The base address of the SMBus io ports and its enabled status is set
>>>>>> by registers in the PCI config space but this was not correctly
>>>>>> emulated. Instead the SMBus registers were mapped on realize to the
>>>>>> base address set by a property to the address expected by fuloong2e
>>>>>> firmware.
>>>>>>
>>>>>> Fix the base and config register handling to more closely model
>>>>>> hardware which allows to remove the property and allows the guest to
>>>>>> control this mapping. Do all this in reset instead of realize so it's
>>>>>> correctly updated on reset.
>>>>>
>>>>> This commit broken running PMON on Fuloong2E:
>>>>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg752605.html
>>>>> console: PMON2000 MIPS Initializing. Standby...
>>>>> console: ERRORPC=00000000 CONFIG=00030932
>>>>> console: PRID=00006302
>>>>> console: DIMM read
>>>>> console: 000000ff
>>>>> console: 000000ff
>>>>> console: 000000ff
>>>>> console: 000000ff
>>>>> console: 000000ff
>>>>> console: 000000ff
>>>>> console: 000000ff
>>>>> console: 000000ff
>>>>> console: 000000ff
>>>>> console: 000000ff
>>>>> ...
>>>>>
>>>>> From here the console loops displaying this value...
>>>>
>>>> Tracing:

>>> pci_cfg_write vt82c686b-pm 05:4 @0x90 <- 0xeee1
>>
>> Offset 93-90 – SMBus I/O Base ....................................... RW
>> 15-4 I/O Base (16-byte I/O space)................ default = 00h
>> pci_cfg_write vt82c686b-pm 05:4 @0x90 <- 0xeee1
>>
>>> pci_cfg_write vt82c686b-pm 05:4 @0xd0 <- 0x1
>>
>> Offset D2 – SMBus Host Configuration ......................... RW
>> SMBus Host Controller Enable
>> 0 Disable SMB controller functions ......... default
>> 1 Enable SMB controller functions
>> pci_cfg_write vt82c686b-pm 05:4 @0xd0 <- 0x1
>>
>> Hmm the datasheet indeed document 0xd2... why is the guest accessing
>> 0xd0 to enable the function? It seems this is the problem, since if
>> I replace d2 -> d0 PMON boots. See below [*].

>>>>> Expected:
>>>>>
>>>>> console: PMON2000 MIPS Initializing. Standby...
>>>>> console: ERRORPC=00000000 CONFIG=00030932
>>>>> console: PRID=00006302
>>>>> console: DIMM read
>>>>> console: 00000080
>>>>> console: read memory type
>>>>> console: read number of rows
>>>>> ...

>>>>>>  static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t
>>>>>> val, int len)
>>>>>>  {
>>>>>> +    VT686PMState *s = VT82C686B_PM(d);
>>>>>> +
>>>>>>      trace_via_pm_write(addr, val, len);
>>>>>>      pci_default_write_config(d, addr, val, len);
>>>>>> +    if (ranges_overlap(addr, len, 0x90, 4)) {
>>>>>> +        uint32_t v = pci_get_long(s->dev.config + 0x90);
>>>>>> +        pci_set_long(s->dev.config + 0x90, (v & 0xfff0UL) | 1);
>>>>>> +    }
>>>>>> +    if (range_covers_byte(addr, len, 0xd2)) {
>>>>>> +        s->dev.config[0xd2] &= 0xf;
>>>>>> +        smb_io_space_update(s);
>>
>> [*] So the guest writing at 0xd0, this block is skipped, the
>> I/O region never enabled.
> 
> Could it be it does word or dword i/o to access multiple addresses at
> once. Wasn't there a recent change to memory regions that could break
> this? Is adjusting valid access sizes to the mem region ops needed now
> to have the memory region handle this?

Do you mean it was buggy earlier, so to accept a guest write at 0xd0
the code had to handle the 0xd2 address? 0xd2 is the address in the
datasheet, so I doubt.