Re: [XEN PATCH] evtchn/fifo: Don't set PENDING bit if guest misbehaves

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> On 16 Mar 2022, at 18:58, Andrew Cooper <amc96@xxxxxxxx> wrote:
> 
> On 16/03/2022 18:38, Raphael Ning wrote:
>> From: Raphael Ning <raphning@xxxxxxxxxx>
>> 
>> Currently, evtchn_fifo_set_pending() will mark the event as PENDING even
>> if it fails to lock the FIFO event queue(s), or if the guest has not
>> initialized the FIFO control block for the target vCPU. A well-behaved
>> guest should never trigger either of these cases.
>> 
>> There is no good reason to set the PENDING bit (the guest should not
>> depend on this behaviour anyway) or check for pollers in such corner
>> cases, so skip that. In fact, both the comment above the for loop and
>> the commit message for
>> 
>> 41a822c39263 xen/events: rework fifo queue locking
>> 
>> suggest that the bit should be set after the FIFO queue(s) are locked.
>> 
>> Take the opportunity to rename the was_pending variable (flipping its
>> sense) and switch to the standard bool type.
>> 
>> Suggested-by: David Vrabel <dvrabel@xxxxxxxxxxxx>
>> Signed-off-by: Raphael Ning <raphning@xxxxxxxxxx>
>> ---
>> xen/common/event_fifo.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/xen/common/event_fifo.c b/xen/common/event_fifo.c
>> index ed4d3beb10f3..6c74ccebebb7 100644
>> --- a/xen/common/event_fifo.c
>> +++ b/xen/common/event_fifo.c
>> @@ -165,7 +165,7 @@ static void cf_check evtchn_fifo_set_pending(
>>     unsigned int port;
>>     event_word_t *word;
>>     unsigned long flags;
>> -    bool_t was_pending;
>> +    bool_t check_pollers = false;
> 
> Considering your commit message, did you intend to change this to bool?
> 
> Can be fixed on commit.  Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> 

I’ve tested on the ARM side, I’ve started/destroyed few guests from Dom0, 
connect to the console, run
some network communications between guest and Dom0, everything works:

Tested-by: Luca Fancellu <luca.fancellu@xxxxxxx>

Cheers,
Luca

> ~Andrew
> 
> P.S. David - do you want your maintainership back?  None of this code
> has undergone any major changes since you wrote it.
>