qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH qemu v4 1/3] spapr_iommu: Realloc guest visible


From: Alexey Kardashevskiy
Subject: Re: [Qemu-devel] [PATCH qemu v4 1/3] spapr_iommu: Realloc guest visible TCE table when hot(un)plugging vfio-pci
Date: Mon, 24 Jul 2017 20:48:45 +1000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1

On 24/07/17 15:53, David Gibson wrote:
> On Thu, Jul 20, 2017 at 05:22:29PM +1000, Alexey Kardashevskiy wrote:
>> This replaces g_malloc() with spapr_tce_alloc_table() as this is
>> the standard way of allocating tables and this allows moving the table
>> back to KVM when unplugging a VFIO PCI device and VFIO TCE acceleration
>> support is not present in the KVM.
> 
> So, I like the idea here, and I think it will work for now, but one
> thing concerns me.
> 
> AIUI, your future plans include allowing in-kernel accelerated TCE
> tables even with VFIO devices.  When that happens, we might have an
> in-kernel table both before and after a change in the "need_vfio"
> state.

The in-kernel table just stays there, mappings will be replayed but in the
end of the replay only the hardware table will change.

> 
> But you won't be able to have two in-kernel tables allocated at once,
> whereas this code relies on having both the old and new tables at once
> so it can copy the contents over.
> 
> How do you intend to handle that?

I intend to make this function no-op when cap_spapr_vfio is present.


> 
>> Although spapr_tce_alloc_table() is expected to fail with EBUSY
>> if called when previous fd is not closed yet, in practice we will not
>> see it because cap_spapr_vfio is false at the moment.
> 
> I don't follow this.  How would cap_spapr_vfio be changing at any
> point?

It depends on the version of the host kernel.

> 
>>
>> Signed-off-by: Alexey Kardashevskiy <address@hidden>
>> ---
>>  hw/ppc/spapr_iommu.c | 35 ++++++++++++++---------------------
>>  1 file changed, 14 insertions(+), 21 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>> index e614621a83..307dc3021e 100644
>> --- a/hw/ppc/spapr_iommu.c
>> +++ b/hw/ppc/spapr_iommu.c
>> @@ -275,33 +275,26 @@ static int spapr_tce_table_realize(DeviceState *dev)
>>  void spapr_tce_set_need_vfio(sPAPRTCETable *tcet, bool need_vfio)
>>  {
>>      size_t table_size = tcet->nb_table * sizeof(uint64_t);
>> -    void *newtable;
>> +    uint64_t *oldtable;
>> +    int newfd = -1;
>>  
>> -    if (need_vfio == tcet->need_vfio) {
>> -        /* Nothing to do */
>> -        return;
>> -    }
>> +    g_assert(need_vfio != tcet->need_vfio);
>>  
>> -    if (!need_vfio) {
>> -        /* FIXME: We don't support transition back to KVM accelerated
>> -         * TCEs yet */
>> -        return;
>> -    }
>> +    tcet->need_vfio = need_vfio;
>>  
>> -    tcet->need_vfio = true;
>> +    oldtable = tcet->table;
>>  
>> -    if (tcet->fd < 0) {
>> -        /* Table is already in userspace, nothing to be do */
>> -        return;
>> -    }
>> +    tcet->table = spapr_tce_alloc_table(tcet->liobn,
>> +                                        tcet->page_shift,
>> +                                        tcet->bus_offset,
>> +                                        tcet->nb_table,
>> +                                        &newfd,
>> +                                        need_vfio);
>> +    memcpy(tcet->table, oldtable, table_size);
>>  
>> -    newtable = g_malloc(table_size);
>> -    memcpy(newtable, tcet->table, table_size);
>> +    spapr_tce_free_table(oldtable, tcet->fd, tcet->nb_table);
>>  
>> -    kvmppc_remove_spapr_tce(tcet->table, tcet->fd, tcet->nb_table);
>> -
>> -    tcet->fd = -1;
>> -    tcet->table = newtable;
>> +    tcet->fd = newfd;
>>  }
>>  
>>  sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn)
> 


-- 
Alexey

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]