Re: [Xen-devel] [PATCH RFC V4 4/5] xen, libxc: Request page fault injection via libxc

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 05.08.14 at 10:09, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 08/04/2014 06:20 PM, Jan Beulich wrote:
>>>>> On 04.08.14 at 17:00, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> On 08/04/2014 05:26 PM, Jan Beulich wrote:
>>>>>>> On 04.08.14 at 13:30, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>>> +    __vmread(VM_ENTRY_INTR_INFO, &ev);
>>>>> +
>>>>> +    if ( (ev & INTR_INFO_VALID_MASK) &&
>>>>> +         hvm_event_needs_reinjection((ev >> 8) & 7, ev & 0xff) )
>>>>
>>>> Are there no manifest constants for all these plain numbers?
>>>
>>> If there are, vmx_vmcs_save() in vmx.c (line 416) doesn't use them. I've
>>> copied that part verbatim.
>> 
>> And that's precisely the problem: As long as there's exactly one use
>> site, the need for manifest constants is questionable (i.e. largely
>> cosmetic). As soon as there are multiple places, connecting them
>> together is largely impossible without naming these numbers - only
>> that way you have a reasonable chance to find the clone of the
>> original should the original be found to need tweaking.
> 
> I'll gladly add #defines for those magic constants, but could you please
> recommend names for them and a header (or at least, category of headers)
> to put them in, in the interest of minimizing the number of RFC versions
> for this series?

Did you even make an attempt to locate a proper place, or
to check whether such constants already exist? Simply looking
for INTR_INFO_VALID_MASK would have turned up

#define INTR_INFO_VECTOR_MASK           0xff            /* 7:0 */
#define INTR_INFO_INTR_TYPE_MASK        0x700           /* 10:8 */

i.e. all you need is there, you just need to make use of them
(and please also - perhaps in an initial cleanup patch - for the
original you cloned from). And just to avoid a needless further
intermediate round: While there is no #define for the shift count
used, you also don't need one if you make use of MASK_EXTR().

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel