Re: [Qemu-devel] [PATCH qemu v6 08/15] spapr_iommu: Introduce "enabled" state for TCE table

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 04/22/2015 04:14 PM, David Gibson wrote:
> On Sat, Apr 11, 2015 at 01:24:37AM +1000, Alexey Kardashevskiy wrote:
>> Currently TCE tables are created once at start and their size never
>> changes. We are going to change that by introducing a Dynamic DMA windows
>> support where DMA configuration may change during the guest execution.
>>
>> This changes spapr_tce_new_table() to create an empty stub object. Only
>> LIOBN is assigned by the time of creation. It still will be called once
>> at the owner object (VIO or PHB) creation.
>>
>> This introduces an "enabled" state for TCE table objects with two
>> helper functions - spapr_tce_table_enable()/spapr_tce_table_disable().
>> spapr_tce_table_enable() receives TCE table parameters and allocates
>> a guest view of the TCE table (in the user space or KVM).
>> spapr_tce_table_disable() disposes the table.
>>
>> Follow up patches will disable+enable tables on reset (system reset
>> or DDW reset).
>>
>> No visible change in behaviour is expected except the actual table
>> will be reallocated every reset. We might optimize this later.
>>
>> The other way to implement this would be dynamically create/remove
>> the TCE table QOM objects but this would make migration impossible
>> as migration expects all QOM objects to exist at the receiver
>> so we have to have TCE table objects created when migration begins.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> Changes:
>> v6:
>> * got rid of set_props()
>> ---
>>   hw/ppc/spapr_iommu.c    | 104 +++++++++++++++++++++++++++++++-----------------
>>   hw/ppc/spapr_pci.c      |  16 +++++---
>>   hw/ppc/spapr_pci_vfio.c |  10 ++---
>>   hw/ppc/spapr_vio.c      |   9 ++---
>>   include/hw/ppc/spapr.h  |  11 ++---
>>   5 files changed, 93 insertions(+), 57 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>> index a14cdc4..64f20f2 100644
>> --- a/hw/ppc/spapr_iommu.c
>> +++ b/hw/ppc/spapr_iommu.c
>> @@ -126,8 +126,47 @@ static MemoryRegionIOMMUOps spapr_iommu_ops = {
>>   static int spapr_tce_table_realize(DeviceState *dev)
>>   {
>>       sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
>> +
>> +    QLIST_INSERT_HEAD(&spapr_tce_tables, tcet, list);
>> +
>> +    vmstate_register(DEVICE(tcet), tcet->liobn, &vmstate_spapr_tce_table,
>> +                     tcet);
>> +
>> +    return 0;
>> +}
>> +
>> +sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn)
>> +{
>> +    sPAPRTCETable *tcet;
>> +    char tmp[64];
>> +
>> +    if (spapr_tce_find_by_liobn(liobn)) {
>> +        fprintf(stderr, "Attempted to create TCE table with duplicate"
>> +                " LIOBN 0x%x\n", liobn);
>> +        return NULL;
>> +    }
>> +
>> +    tcet = SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE));
>> +    tcet->liobn = liobn;
>> +
>> +    snprintf(tmp, sizeof(tmp), "tce-table-%x", liobn);
>> +    object_property_add_child(OBJECT(owner), tmp, OBJECT(tcet), NULL);
>> +
>> +    object_property_set_bool(OBJECT(tcet), true, "realized", NULL);
>> +
>> +    trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd);
>> +
>> +    return tcet;
>> +}
>> +
>> +static void spapr_tce_table_do_enable(sPAPRTCETable *tcet)
>
> AFAICT there's only one caller of this, so it's not clear why this
> isn't just open-coded in spapr_tce_table_enable().


There is another call in "[PATCH qemu v6 14/15] spapr_pci/spapr_pci_vfio: 
Support Dynamic DMA Windows (DDW)", in spapr_tce_table_post_load().

Should I put a note in the commit log?


-- 
Alexey