Re: [PATCH 5/5] xen: Write CR0, CR3 and CR4 in arch_set_info_guest()

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 05/18/2015 11:05 AM, Jan Beulich wrote:
>>>> On 18.05.15 at 09:58, <rcojocaru@bitdefender.com> wrote:
>> On 05/18/2015 10:27 AM, Jan Beulich wrote:
>>>>>> On 15.05.15 at 22:45, <rcojocaru@bitdefender.com> wrote:
>>>> On 05/15/2015 06:57 PM, Jan Beulich wrote:
>>>>>>>> On 06.05.15 at 19:12, <rcojocaru@bitdefender.com> wrote:
>>>>>> Arch_set_info_guest() doesn't set CR0, CR3 or CR4. Added code
>>>>>> that does that.
>>>>>
>>>>> But you should also say a word on why this is needed, since things
>>>>> worked fine so far without, and enabling the functions to run
>>>>> outside of their own vCPU context is not immediately obviously
>>>>> correct.
>>>>
>>>> This is a way to undo malicious CR writes. This is achieved for MSR
>>>> writes with the deny vm_event response flag patch in this series, but
>>>> the CR events are being send after the actual write. In such cases,
>>>> while the VCPU is paused before I put a vm_response in the ring, I can
>>>> simply write the old value back.
>>>>
>>>> I've brought up the issue in the past, and the consensus, IIRC, was that
>>>> I should not alter existing behaviour (post-write events) - so the
>>>> alternatives were either to add a new pre-write CR event (which seemed
>>>> messy), or this (which seemed less intrusive).
>>>>
>>>> Of course, if it has now become acceptable to reconsider having the CR
>>>> vm_events consistently pre-write, the deny patch could be extended to them.
>>>
>>> Considering that in the reply to Andrew's response you already
>>> pointed at where a suggestion towards consistent pre events was
>>> made, it would have helped if you identified where this was advised
>>> against before. It certainly seems sensible to treat MSR and CR
>>> writes in a consistent fashion.
>>
>> Sorry for the omission. This is the original reply:
>>
>> http://lists.xen.org/archives/html/xen-devel/2014-10/msg00240.html 
>>
>> where you've pointed out that we should not break existing behaviour.
> 
> As said (mainly by Andrew) numerous times recently, breaking of
> existing behavior is - with all the incompatible changes already done
> to the interface after 4.5 - not currently an issue.
> 
>> 2. Change the control register events to pre-write and prevent
>> post-write events in the future.
> 
> This one, as long as amongst the ones that care for the events
> agreement can be reached that this is the way to go.

Ack, unless otherwise requested I'll add this modification to the next
iteration of "xen/vm_event: Clean up control-register-write vm_events".


Thanks,
Razvan