On 06.09.21 13:38, Julien Grall wrote:
> Hi Oleksandr,
>
> On 06/09/2021 11:06, Oleksandr Andrushchenko wrote:
>> On 06.09.21 12:53, Julien Grall wrote:
>>>>>>> However, looking at the rest of the code, we already have a check for
>>>>>>> vpci in the common IOREQ code.
>>>>>>
>>>>>> Which may not be enabled as it depends on CONFIG_IOREQ_SERVER.
>>>>>
>>>>> Right. My point is when CONFIG_IOREQ_SERVER is set then you would end up
>>>>> to call twice vpci_process_pending(). This will have an impact how on
>>>>> long your vCPU is going to running because you are doubling the work.
>>>>
>>>> So, you suggest that we have in the common IOREQ code something call like
>>>>
>>>> arch_vpci_process_pending? In case of x86 it will have the code currently
>>>> found in the
>>>>
>>>> common IOREQ sources and for Arm it will be nop?
>>>
>>> No I am suggesting to move the call of the IOREQ code to hvm_do_resume()
>>> (on x86) and check_for_vcpu_work() (on Arm).
>>
>> Ok, I can move vPCI code to hvm_do_resume, but vPCI is only used for x86 PVH
>> Dom0.
>
> AFAIK, Roger is planning to use it for x86 PVH guest.
>
>>
>> Do you still think hvm_do_resume is the right place?
> I think so. AFAICT, on x86, the only caller of vcpu_ioreq_handle_completion()
> is hvm_do_resume(). So it makes sense to push one layer up.
Ok, so I ended up with:
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2305,10 +2305,17 @@ static bool check_for_vcpu_work(void)
}
#endif
- local_irq_enable();
- if ( has_vpci(v->domain) && vpci_process_pending(v) )
- raise_softirq(SCHEDULE_SOFTIRQ);
- local_irq_disable();
+ if ( has_vpci(v->domain) )
+ {
+ bool pending;
+
+ local_irq_enable();
+ pending = vpci_process_pending(v);
+ local_irq_disable();
+
+ if ( pending )
+ return true;
+ }
This is how it is done for IOREQ. It seems there is no need to raise softirq.
I also moved vPCI from common code to hvm_do_resume for x86
>
> Cheers,
>
Thanks,
Oleksandr
