Re: [PATCH v2 for-4.14 1/3] xen/monitor: Control register values

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Tue, Jun 2, 2020 at 6:47 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 02.06.2020 14:40, Tamas K Lengyel wrote:
> > On Tue, Jun 2, 2020 at 5:08 AM Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
> >>
> >> On Wed, May 20, 2020 at 08:31:52PM -0600, Tamas K Lengyel wrote:
> >>> Extend the monitor_op domctl to include option that enables
> >>> controlling what values certain registers are permitted to hold
> >>> by a monitor subscriber.
> >>
> >> I think the change could benefit for some more detail commit message
> >> here. Why is this useful?
> >
> > You would have to ask the Bitdefender folks who made the feature. I
> > don't use it. Here we are just making it optional as it is buggy so it
> > is disabled by default.
>
> Now that's exactly the opposite of what I had derived from the
> description here so far. Perhaps an at least weak indication
> that you want to reword this. For example, from your reply to
> Roger I understand it's rather that the new flag allows to
> "suppress" the controlling (since presumably you don't change
> default behavior), rather then "enabling" it.

What we are adding is a domctl you need to call that enables this
feature. It's not an option to suppress it. It shouldn't have been
enabled by default to begin with. That was a mistake when the feature
was contributed and it is buggy.

>
> >> There already seems to be some support for gating MSR writes, which
> >> seems to be expanded by this commit?
> >
> > We don't expand on any existing features, we make an existing feature 
> > optional.
> >
> >>
> >> Is it solving some kind of bug reported?
> >
> > It does, please take a look at the cover letter.
>
> Please can such important aspects also go in the commit message,
> so they're available later without needing to hunt down mail
> threads (besides this way being more readily available to
> reviewers)?

Noted.

Thanks,
Tamas