Re: [PATCH V3 1/3] xen/mem_access: Support for memory-content hiding

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 07/07/2015 06:40 PM, Jan Beulich wrote:
>>>> On 07.07.15 at 17:32, <rcojocaru@bitdefender.com> wrote:
>> On 07/07/2015 04:27 PM, Jan Beulich wrote:
>>>>>> On 06.07.15 at 17:51, <rcojocaru@bitdefender.com> wrote:
>>>> @@ -1552,9 +1556,15 @@ bool_t p2m_mem_access_check(paddr_t gpa, unsigned long gla,
>>>>  
>>>>      if ( v->arch.vm_event.emulate_flags )
>>>>      {
>>>> -        hvm_mem_access_emulate_one((v->arch.vm_event.emulate_flags &
>>>> -                                    MEM_ACCESS_EMULATE_NOWRITE) != 0,
>>>> -                                   TRAP_invalid_op, HVM_DELIVER_NO_ERROR_CODE);
>>>> +        enum emul_kind kind = EMUL_KIND_NORMAL;
>>>> +
>>>> +        if ( v->arch.vm_event.emulate_flags & MEM_ACCESS_SET_EMUL_READ_DATA )
>>>> +            kind = EMUL_KIND_SET_CONTEXT;
>>>> +        else if ( v->arch.vm_event.emulate_flags & MEM_ACCESS_EMULATE_NOWRITE )
>>>
>>> Is there code in place rejecting both flags being set at once? I don't
>>> recall having seen any...
>>
>> No, there isn't. Both flags can be set at once, but if so only the
>> SET_EMUL_READ_DATA will be honored.
> 
> But to me, purely theoretically setting both flags together makes
> sense, and hence this combination, if it isn't working today,
> shouldn't result in unexpected behavior (perhaps differing from
> what a future Xen version might do).

That's a very good point. I have tried to be as clear about this as
possible in the code by using an enum for the possible emulation kinds,
instead of #defines or something potentially interpretable as bit
setters. But due to the design of the vm_event mechanism it's not
possible to return an error when the response contains OR-ed bits that
don't belong together.

The behaviour now is that NOWRITE outranks the plain EMULATE, and
SET_CONTEXT outranks them both, and there are no plans to change this in
the future. I suppose a comment stating this clearly belongs in vm_event.h?


Thanks,
Razvan