Hi Jan,
On 6 March 2017 at 13:45, Jan Beulich <jbeul...@suse.com> wrote:
>>>> On 05.03.17 at 13:39, <julien.gr...@arm.com> wrote:
>> My knowledge is limited for this code. So I've just CCed "The REST"
>> maintainers. Please do CC them in the future.
>
> Indeed.
>
I will take care of this in the future.
>> On 02/21/2017 11:26 AM, Bhupinder Thakur wrote:
>>> Breakup evtchn_send() to allow sending events for a Xen bound channel.
>>> Currently,
>>> there is a check in evtchn_send() i.e. is_consumer_xen() that if the event
>>> channel
>>> is bound to a xen consumer then event generation is not allowed for that
>>> channel.
>>> This check is to disallow a guest from raising an event via this channel.
>>> However,
>>> it should allow Xen to send a event via this channel as it is required for
>>> sending
>>> vpl011 event to the dom0.
>>>
>>> This change introduces a new function raw_evtchn_send() which sends the
>>> event
>>> without this check. The current evtchn_send() calls this function after
>>> doing the
>>> xen consumer check. Xen uses the raw_evtchm_send() version to send the
>>> event thus
>>> bypassing the check.
>
> Why would Xen want to send an event it is itself the consumer of?
> Surely there are better ways to communicate state internally? The
> more that you say you want the event sent to Dom0...
>
As a consumer, Xen receives event from dom0. It also needs to send
events to dom0 to indicate that there is data in the ring buffer for
dom0 to read. I am using a xen bound event channel for
sending/receiving events to/from dom0. I added a new function
raw_evtchn_send() to allow Xen to send events to dom0 without doing
the is_xen_consumer check. Note that this check is still there in
evtchn_send() to disallow guests to raise events on the xen bound
channel.
>>> @@ -650,25 +651,21 @@ static long evtchn_close(struct domain *d1, int
>>> port1, bool_t guest)
>>> return rc;
>>> }
>>>
>>> -int evtchn_send(struct domain *ld, unsigned int lport)
>>> +int raw_evtchn_send(struct domain *ld, unsigned int lport, void *data)
>>> {
>>> struct evtchn *lchn, *rchn;
>>> struct domain *rd;
>>> - int rport, ret = 0;
>>> + int rport, ret=0;
>
> There's only whitespace change here, and just the break existing
> formatting. Please avoid stray changes like this.
>
I will fix this in the next version.
>>> @@ -696,6 +693,32 @@ int evtchn_send(struct domain *ld, unsigned int lport)
>>> }
>>>
>>> out:
>>> + if ( !data )
>>> + spin_unlock(&lchn->lock);
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +int evtchn_send(struct domain *ld, unsigned int lport)
>>> +{
>>> + struct evtchn *lchn;
>>> + int ret;
>>> +
>>> + if ( !port_is_valid(ld, lport) )
>>> + return -EINVAL;
>>> +
>>> + lchn = evtchn_from_port(ld, lport);
>>> +
>>> + spin_lock(&lchn->lock);
>>> +
>>> + if ( unlikely(consumer_is_xen(lchn)) )
>>> + {
>>> + printk("evtchn_send failed to send via xen event channel\n");
>>> + return -EINVAL;
>
> As a general remark: Splitting out functionality into a new function
> should _never_ be accompanied by silent behavioral changes for
> pre-existing code paths. In the case here there was no printk()
> before, and I see no justification for one to be added.
>
Ok. I will remove the printk.
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
