Re: [PATCH v5 10/10] xen/arm: Process pending vPCI map/unmap operations

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

Hi, Julien!

On 13.10.21 18:30, Julien Grall wrote:
> Hi Oleksandr,
>
> On 08/10/2021 06:55, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>>
>> vPCI may map and unmap PCI device memory (BARs) being passed through which
>> may take a lot of time. For this those operations may be deferred to be
>> performed later, so that they can be safely preempted.
>>
>> Currently this deferred processing is happening in common IOREQ code
>> which doesn't seem to be the right place for x86 and is even more
>> doubtful because IOREQ may not be enabled for Arm at all.
>> So, for Arm the pending vPCI work may have no chance to be executed
>> if the processing is left as is in the common IOREQ code only.
>> For that reason make vPCI processing happen in arch specific code.
>>
>> Please be aware that there are a few outstanding TODOs affecting this
>> code path, see xen/drivers/vpci/header.c:map_range and
>> xen/drivers/vpci/header.c:vpci_process_pending.
>>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>> [x86 changes]
>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>> Reviewed-by: Rahul Singh <rahul.singh@xxxxxxx>
>> Tested-by: Rahul Singh <rahul.singh@xxxxxxx>
>> ---
>> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> Cc: Paul Durrant <paul@xxxxxxx>
>>
>> Since v2:
>>   - update commit message with more insight on x86, IOREQ and Arm
>>   - restored order of invocation for IOREQ and vPCI processing (Jan)
>> Since v1:
>>   - Moved the check for pending vpci work from the common IOREQ code
>>     to hvm_do_resume on x86
>>   - Re-worked the code for Arm to ensure we don't miss pending vPCI work
>> ---
>>   xen/arch/arm/traps.c   | 13 +++++++++++++
>>   xen/arch/x86/hvm/hvm.c |  6 ++++++
>>   xen/common/ioreq.c     |  9 ---------
>>   3 files changed, 19 insertions(+), 9 deletions(-)
>>
>> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
>> index 219ab3c3fbde..b246f51086e3 100644
>> --- a/xen/arch/arm/traps.c
>> +++ b/xen/arch/arm/traps.c
>> @@ -34,6 +34,7 @@
>>   #include <xen/symbols.h>
>>   #include <xen/version.h>
>>   #include <xen/virtual_region.h>
>> +#include <xen/vpci.h>
>>     #include <public/sched.h>
>>   #include <public/xen.h>
>> @@ -2304,6 +2305,18 @@ static bool check_for_vcpu_work(void)
>>       }
>>   #endif
>>   +    if ( has_vpci(v->domain) )
>> +    {
>> +        bool pending;
>> +
>> +        local_irq_enable();
>> +        pending = vpci_process_pending(v);
>> +        local_irq_disable();
>> +
>> +        if ( pending )
>> +            return true;
>> +    }
>> +
>
> I would prefer if this addition is moved before the 
> vcpu_ioreq__handle_completion(v). This is to avoid differences with the x86 
> version.
Ok, will do

Thanks,
Oleksandr