Re: [Qemu-devel] [PATCH 1/8] spapr-iommu: add a bus for spapr-iommu devices

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 04/10/2014 10:40 PM, Alexander Graf wrote:
> 
> On 14.03.14 05:18, Alexey Kardashevskiy wrote:
>> At the moment sPAPR IOMMU table is a device which participates in
>> a migration stream. Normally QEMU uses a get_dev_path() hook from
>> the device's bus to compose the section name and @instance_id which are
>> used to match the section to the real device. This works till the user
>> changes the device order in the command line - if this happens,
>> devices get other instance_id's and migration fails.
>>
>> This adds a TCE bridge bus device per sPAPR machine and places all sPAPR
>> IOMMU devices onto it.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> Juan, is a different command line device order supposed to work with
> migration?


We discussed this on IRC with Paolo and the conclusion is that yes, the
order should not matter.

Ideally we should implement "irq" property for every device (and INTA/B/C/D
for PHB) and run the source and destination QEMU with exact IRQ numbers (of
nail IRQ numbers to devices somehow?). But for me either is overkill.


> 
> Alex
> 
>> ---
>>   hw/ppc/spapr.c         |  3 +++
>>   hw/ppc/spapr_iommu.c   | 59
>> +++++++++++++++++++++++++++++++++++++++++++++++++-
>>   include/hw/ppc/spapr.h |  7 ++++++
>>   3 files changed, 68 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 5c9a154..12adc21 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1263,6 +1263,9 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>       /* Set up EPOW events infrastructure */
>>       spapr_events_init(spapr);
>>   +    /* Set up TCE IOMMUs bus */
>> +    spapr->tce_bus = spapr_tce_bus_init();
>> +
>>       /* Set up VIO bus */
>>       spapr->vio_bus = spapr_vio_bus_init();
>>   diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>> index d9fe946..7db0acf 100644
>> --- a/hw/ppc/spapr_iommu.c
>> +++ b/hw/ppc/spapr_iommu.c
>> @@ -157,7 +157,7 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState
>> *owner, uint32_t liobn, size_t wi
>>           return NULL;
>>       }
>>   -    tcet = SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE));
>> +    tcet = SPAPR_TCE_TABLE(qdev_create(spapr->tce_bus,
>> TYPE_SPAPR_TCE_TABLE));
>>       tcet->liobn = liobn;
>>       tcet->window_size = window_size;
>>   @@ -342,9 +342,66 @@ static TypeInfo spapr_tce_table_info = {
>>       .instance_finalize = spapr_tce_table_finalize,
>>   };
>>   +static char *spapr_tce_bus_get_dev_name(DeviceState *qdev)
>> +{
>> +    sPAPRTCETable *tcet = SPAPR_TCE_TABLE(qdev);
>> +    char *name;
>> +
>> +    name = g_strdup_printf("liobn@%x", tcet->liobn);
>> +    return name;
>> +}
>> +
>> +static void spapr_tce_bus_class_init(ObjectClass *klass, void *data)
>> +{
>> +    BusClass *k = BUS_CLASS(klass);
>> +
>> +    k->get_dev_path = spapr_tce_bus_get_dev_name;
>> +}
>> +
>> +static const TypeInfo spapr_tce_bus_info = {
>> +    .name = TYPE_SPAPR_TCE_BUS,
>> +    .parent = TYPE_BUS,
>> +    .class_init = spapr_tce_bus_class_init,
>> +    .instance_size = sizeof(BusState),
>> +};
>> +
>> +static int spapr_tce_bridge_init(SysBusDevice *dev)
>> +{
>> +    /* nothing */
>> +    return 0;
>> +}
>> +
>> +static void spapr_tce_bridge_class_init(ObjectClass *klass, void *data)
>> +{
>> +    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>> +
>> +    k->init = spapr_tce_bridge_init;
>> +}
>> +
>> +static const TypeInfo spapr_tce_bridge_info = {
>> +    .name          = "spapr-tce-bridge",
>> +    .parent        = TYPE_SYS_BUS_DEVICE,
>> +    .instance_size = sizeof(SysBusDevice),
>> +    .class_init    = spapr_tce_bridge_class_init,
>> +};
>> +
>>   static void register_types(void)
>>   {
>>       type_register_static(&spapr_tce_table_info);
>> +    type_register_static(&spapr_tce_bridge_info);
>> +    type_register_static(&spapr_tce_bus_info);
>> +}
>> +
>> +BusState *spapr_tce_bus_init(void)
>> +{
>> +    DeviceState *dev;
>> +
>> +    /* Create bridge device */
>> +    dev = qdev_create(NULL, spapr_tce_bridge_info.name);
>> +    qdev_init_nofail(dev);
>> +
>> +    /* Create bus on bridge device */
>> +    return qbus_create(TYPE_SPAPR_TCE_BUS, dev, "spapr-tce");
>>   }
>>     type_init(register_types);
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index 449fc7c..18332fd 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -12,6 +12,7 @@ struct sPAPRNVRAM;
>>     typedef struct sPAPREnvironment {
>>       struct VIOsPAPRBus *vio_bus;
>> +    BusState *tce_bus;
>>       QLIST_HEAD(, sPAPRPHBState) phbs;
>>       hwaddr msi_win_addr;
>>       MemoryRegion msiwindow;
>> @@ -405,4 +406,10 @@ int spapr_dma_dt(void *fdt, int node_off, const char
>> *propname,
>>   int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname,
>>                         sPAPRTCETable *tcet);
>>   +#define TYPE_SPAPR_TCE_BUS "spapr-tce-bus"
>> +#define SPAPR_TCE_BUS(obj) \
>> +    OBJECT_CHECK(BusState, (obj), TYPE_SPAPR_TCE_BUS)
>> +
>> +BusState *spapr_tce_bus_init(void);
>> +
>>   #endif /* !defined (__HW_SPAPR_H__) */
> 


-- 
Alexey