Re: [PATCH 2/7] xen/events: don't unmask an event channel when an eoi is pending

From: Ross Lagerwall <ross.lagerwall@citrix.com>
To: Juergen Gross <jgross@suse.com>, <xen-devel@lists.xenproject.org>,
	<linux-kernel@vger.kernel.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	<stable@vger.kernel.org>, Julien Grall <julien@xen.org>
Subject: Re: [PATCH 2/7] xen/events: don't unmask an event channel when an eoi is pending
Date: Mon, 8 Feb 2021 10:15:31 +0000	[thread overview]
Message-ID: <bdf20a87-5b4d-529e-7028-cea3eeef769b@citrix.com> (raw)
In-Reply-To: <20210206104932.29064-3-jgross@suse.com>

On 2021-02-06 10:49, Juergen Gross wrote:
> An event channel should be kept masked when an eoi is pending for it.
> When being migrated to another cpu it might be unmasked, though.
> 
> In order to avoid this keep two different flags for each event channel
> to be able to distinguish "normal" masking/unmasking from eoi related
> masking/unmasking. The event channel should only be able to generate
> an interrupt if both flags are cleared.
> 
> Cc: stable@vger.kernel.org
> Fixes: 54c9de89895e0a36047 ("xen/events: add a new late EOI evtchn framework")
> Reported-by: Julien Grall <julien@xen.org>
> Signed-off-by: Juergen Gross <jgross@suse.com>
...> +static void lateeoi_ack_dynirq(struct irq_data *data)
> +{
> +	struct irq_info *info = info_for_irq(data->irq);
> +	evtchn_port_t evtchn = info ? info->evtchn : 0;
> +
> +	if (VALID_EVTCHN(evtchn)) {
> +		info->eoi_pending = true;
> +		mask_evtchn(evtchn);
> +	}
> +}

Doesn't this (and the one below) need a call to clear_evtchn() to
actually ack the pending event? Otherwise I can't see what clears
the pending bit.

I tested out this patch but processes using the userspace evtchn device did
not work very well without the clear_evtchn() call.

Ross

> +
> +static void lateeoi_mask_ack_dynirq(struct irq_data *data)
> +{
> +	struct irq_info *info = info_for_irq(data->irq);
> +	evtchn_port_t evtchn = info ? info->evtchn : 0;
> +
> +	if (VALID_EVTCHN(evtchn)) {
> +		info->masked = true;
> +		info->eoi_pending = true;
> +		mask_evtchn(evtchn);
> +	}
> +}
> +