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

[Julien Grall <julien@xen.org>]

Hi Oleksandr,

On 03/09/2021 09:33, Oleksandr Andrushchenko wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> 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.
> Run the corresponding vPCI code while switching a vCPU.

IIUC, you are talking about the function map_range() in 
xen/drivers/vpci/header. The function has the following todo for Arm:

         /*
          * ARM TODOs:
          * - On ARM whether the memory is prefetchable or not should be 
passed
          *   to map_mmio_regions in order to decide which memory attributes
          *   should be used.
          *
          * - {un}map_mmio_regions doesn't support preemption.
          */

This doesn't seem to be addressed in the two series for PCI passthrough 
sent so far. Do you have any plan to handle it?

> 
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> ---
>   xen/arch/arm/traps.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 219ab3c3fbde..1571fb8afd03 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,11 @@ static bool check_for_vcpu_work(void)
>       }
>   #endif
>   
> +    local_irq_enable();
> +    if ( has_vpci(v->domain) && vpci_process_pending(v) )

Looking at the code of vpci_process_pending(), it looks like there are 
some rework to do for guest. Do you plan to handle it as part of the 
vPCI series?

> +        raise_softirq(SCHEDULE_SOFTIRQ);
> +    local_irq_disable();
> +

 From my understanding of vcpi_process_pending(). The function will 
return true if there are more work to schedule. However, if 
check_for_vcpu_for_work() return false, then we will return to the guest 
before any work for vCPI has finished. This is because 
check_for_vcpu_work() will not be called again.

In this case, I think you want to return as soon as you know we need to 
reschedule.

However, looking at the rest of the code, we already have a check for 
vpci in the common IOREQ code. So we would end up to call twice 
vpci_process_pending(). Maybe we should move the call from the IOREQ to 
arch-code.

Cheers,

-- 
Julien Grall