Re: [Qemu-ppc] [PATCH 4/8] tests: Better handle legacy IO addresses in tco-test

[Laurent Vivier]

On 18/10/2016 17:14, Laurent Vivier wrote:
> 
> 
> On 18/10/2016 12:52, David Gibson wrote:
>> tco_test uses the libqos PCI code to access the device.  This makes perfect
>> sense for the PCI config space accesses.  However for IO, rather than the
>> usual PCI approach of mapping a PCI BAR, then accessing that, it instead
>> uses the legacy approach of fixed, known addresses in PCI IO space.
>>
>> That doesn't work very well with the qpci_io_{read,write} functions because
>> we never use qpci_iomap() and so have to make assumptions about the
>> internal encoding of the address tokens iomap() returns.
>>
>> This patch avoids that, by directly using the bus's pio_{read,write}
>> callbacks, which are defined to take addresses within the PCI IO space.
>>
>> Signed-off-by: David Gibson <address@hidden>
>> ---
>>  tests/tco-test.c | 87 
>> ++++++++++++++++++++++++++++----------------------------
>>  1 file changed, 44 insertions(+), 43 deletions(-)
>>
>> diff --git a/tests/tco-test.c b/tests/tco-test.c
>> index 0d201b1..e668630 100644
>> --- a/tests/tco-test.c
>> +++ b/tests/tco-test.c
>> @@ -40,13 +40,13 @@ enum {
>>  typedef struct {
>>      const char *args;
>>      bool noreboot;
>> +    QPCIBus *bus;
>>      QPCIDevice *dev;
>> -    void *tco_io_base;
>> +    uint16_t tco_io_base;
>>  } TestData;
>>  
>>  static void test_init(TestData *d)
>>  {
>> -    QPCIBus *bus;
>>      QTestState *qs;
>>      char *s;
>>  
>> @@ -57,8 +57,8 @@ static void test_init(TestData *d)
>>      qtest_irq_intercept_in(qs, "ioapic");
>>      g_free(s);
>>  
>> -    bus = qpci_init_pc(NULL);
>> -    d->dev = qpci_device_find(bus, QPCI_DEVFN(0x1f, 0x00));
>> +    d->bus = qpci_init_pc(NULL);
> 
> You can use qtest_pc_boot() now.
> 
>> +    d->dev = qpci_device_find(d->bus, QPCI_DEVFN(0x1f, 0x00));
>>      g_assert(d->dev != NULL);
>>  
>>      qpci_device_enable(d->dev);
>> @@ -70,42 +70,42 @@ static void test_init(TestData *d)
>>      /* set Root Complex BAR */
>>      qpci_config_writel(d->dev, ICH9_LPC_RCBA, RCBA_BASE_ADDR | 0x1);
>>  
>> -    d->tco_io_base = (void *)((uintptr_t)PM_IO_BASE_ADDR + 0x60);
>> +    d->tco_io_base = PM_IO_BASE_ADDR + 0x60;
> 
> Why don't you use QPCIBar in TestData to store the address?
> And you can call qpci_io_XXX() with it.

OK, I was watching the state of qpci_io_XXX() after the series, so you
can't do that here, but perhaps this patch should be moved to PATCH 8/8?

Laurent