From mboxrd@z Thu Jan 1 00:00:00 1970 From: Corneliu ZUZU Subject: Re: [PATCH 11/16] x86/monitor: fix: treat -monitor- properly, as a subsys of the vm-event subsys Date: Tue, 12 Jul 2016 00:47:05 +0300 Message-ID: References: <1468037509-6428-1-git-send-email-czuzu@bitdefender.com> <1468038011-6935-1-git-send-email-czuzu@bitdefender.com> <25a33c3f-0465-494c-51c5-abe8b56c85f7@bitdefender.com> <2c2b48d6-c0d8-71c8-8bb6-47690ce5e959@bitdefender.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3185722657547939680==" Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Tamas K Lengyel Cc: Stefano Stabellini , Razvan Cojocaru , Andrew Cooper , Xen-devel , Julien Grall , Paul Durrant , Jan Beulich List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --===============3185722657547939680== Content-Type: multipart/alternative; boundary="------------4117A380C86DF8D44639CDC9" This is a multi-part message in MIME format. --------------4117A380C86DF8D44639CDC9 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 7/12/2016 12:27 AM, Tamas K Lengyel wrote: > On Mon, Jul 11, 2016 at 2:20 PM, Corneliu ZUZU wrote: >> On 7/11/2016 7:38 PM, Tamas K Lengyel wrote: >>>>>> diff --git a/xen/include/asm-arm/monitor.h >>>>>> b/xen/include/asm-arm/monitor.h >>>>>> index 9a9734a..7ef30f1 100644 >>>>> [snip] >>>> >>>> I keep seeing '[snip]' lately but I don't know what it means. >>> Placeholder for "I've cut some of your patch content here to shorten >>> the message I'm sending". >>> >>>>>> diff --git a/xen/include/xen/monitor.h b/xen/include/xen/monitor.h >>>>>> index 2171d04..605caf0 100644 >>>>>> --- a/xen/include/xen/monitor.h >>>>>> +++ b/xen/include/xen/monitor.h >>>>>> @@ -22,12 +22,15 @@ >>>>>> #ifndef __XEN_MONITOR_H__ >>>>>> #define __XEN_MONITOR_H__ >>>>>> >>>>>> -#include >>>>>> - >>>>>> -struct domain; >>>>>> -struct xen_domctl_monitor_op; >>>>>> +#include >>>>>> >>>>>> int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op >>>>>> *op); >>>>>> + >>>>>> +static inline bool_t monitor_domain_initialised(const struct domain >>>>>> *d) >>>>>> +{ >>>>>> + return d->monitor.initialised; >>>>> This should be !!d->monitor.initialised. >>>> >>>> It's the value of a bit, thus should be 0 or 1. Am I missing something? >>> Using a bitfield is effectively doing a bitmask for the bit(s). >>> However, returning the value after applying a bitmask is not >>> necessarily 0/1 (ie. bool_t). For example I ran into problems with >>> this in the past (see >>> https://lists.xen.org/archives/html/xen-devel/2015-08/msg01948.html). >>> >>> Tamas >> >> The example you provided actually returns a value &-ed with a flag >> (bitmask), of course it returns either 0 or the flag itself :-). AFAIK a >> single-bit item in a bitfield (note a -bitfield-, not e.g. an unsigned int >> &-ed with (1<> > I would assume as well but I'm not actually sure if that's guaranteed > or if it's only compiler defined behavior. > > Tamas > As per http://en.cppreference.com/w/c/language/bit_field . "The number of bits in a bit field (width) sets the limit to the range of values it can hold" So if it's width is 1, it can either be 0 or 1. Corneliu. --------------4117A380C86DF8D44639CDC9 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 7bit
On 7/12/2016 12:27 AM, Tamas K Lengyel wrote:
On Mon, Jul 11, 2016 at 2:20 PM, Corneliu ZUZU <czuzu@bitdefender.com> wrote:

On 7/11/2016 7:38 PM, Tamas K Lengyel wrote:

diff --git a/xen/include/asm-arm/monitor.h
b/xen/include/asm-arm/monitor.h
index 9a9734a..7ef30f1 100644

[snip]


I keep seeing '[snip]' lately but I don't know what it means.

Placeholder for "I've cut some of your patch content here to shorten
the message I'm sending".


diff --git a/xen/include/xen/monitor.h b/xen/include/xen/monitor.h
index 2171d04..605caf0 100644
--- a/xen/include/xen/monitor.h
+++ b/xen/include/xen/monitor.h
@@ -22,12 +22,15 @@
   #ifndef __XEN_MONITOR_H__
   #define __XEN_MONITOR_H__

-#include <public/vm_event.h>
-
-struct domain;
-struct xen_domctl_monitor_op;
+#include <xen/sched.h>

   int monitor_domctl(struct domain *d, struct xen_domctl_monitor_op
*op);
+
+static inline bool_t monitor_domain_initialised(const struct domain
*d)
+{
+    return d->monitor.initialised;

This should be !!d->monitor.initialised.


It's the value of a bit, thus should be 0 or 1. Am I missing something?

Using a bitfield is effectively doing a bitmask for the bit(s).
However, returning the value after applying a bitmask is not
necessarily 0/1 (ie. bool_t). For example I ran into problems with
this in the past (see
https://lists.xen.org/archives/html/xen-devel/2015-08/msg01948.html).

Tamas


The example you provided actually returns a value &-ed with a flag
(bitmask), of course it returns either 0 or the flag itself :-). AFAIK a
single-bit item in a bitfield (note a -bitfield-, not e.g. an unsigned int
&-ed with (1<<x)) will always be either 0 or 1.


I would assume as well but I'm not actually sure if that's guaranteed
or if it's only compiler defined behavior.

Tamas

As per http://en.cppreference.com/w/c/language/bit_field .

"The number of bits in a bit field (width) sets the limit to the range of values it can hold"

So if it's width is 1, it can either be 0 or 1.

Corneliu.
--------------4117A380C86DF8D44639CDC9-- --===============3185722657547939680== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============3185722657547939680==--