Re: [Xen-devel] [PATCH v2 1/2] x86/monitor: add masking support for write_ctrlreg events

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Fri, Jun 16, 2017 at 9:33 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 16.06.17 at 17:28, <tamas@xxxxxxxxxxxxx> wrote:
>> On Fri, Jun 16, 2017 at 9:15 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>> On 16.06.17 at 16:26, <tamas@xxxxxxxxxxxxx> wrote:
>>>> On Tue, May 30, 2017 at 3:46 AM, Petre Pircalabu
>>>> <ppircalabu@xxxxxxxxxxxxxxx> wrote:
>>>>> Add support for filtering out the write_ctrlreg monitor events if they
>>>>> are generated only by changing certains bits.
>>>>> A new parameter (bitmask) was added to the xc_monitor_write_ctrlreg
>>>>> function in order to mask the event generation if the changed bits are
>>>>> set.
>>>>>
>>>>> Signed-off-by: Petre Pircalabu <ppircalabu@xxxxxxxxxxxxxxx>
>>>>
>>>> Acked-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
>>>
>>> Are you btw in agreement with ...
>>>
>>>>> @@ -1107,6 +1107,11 @@ struct xen_domctl_monitor_op {
>>>>>              uint8_t sync;
>>>>>              /* Send event only on a change of value */
>>>>>              uint8_t onchangeonly;
>>>>> +            /*
>>>>> +             * Send event only if the changed bit in the control register
>>>>> +             * is not masked.
>>>>> +             */
>>>>> +            uint64_aligned_t bitmask;
>>>
>>> ... the 5-byte gap being introduced here, using of which won't
>>> be possible without bumping XEN_DOMCTL_INTERFACE_VERSION
>>> again? Generally we aim at making padding explicit and checking
>>> it to be zero on input / providing zero on output.
>>
>> Yes, making the padding explicit would be better (but I'm not the
>> maintainer of the domctl interface so my ack doesn't cover that).
>
> Well, I have been waiting for a monitor maintainer ack to perhaps
> give my own for the little bit of it where a REST maintainer one
> would be needed. Irrespective of file level maintainership I think
> the public interface parts still belong to their subsystems, and the
> REST maintainer ack should normally be a purely mechanical last
> step.

I can see the logic behind that. However, I would feel better if it
wasn't just a mechanical last step as for example I'm not as well
versed in the ABI side of things. Like in here, the padding bits have
completely went by me without me noticing.

Tamas

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