Re: [PATCH V5 3/3] xen/vm_event: Deny register writes if refused by vm_event reply

["Jan Beulich" <JBeulich@suse.com>]

>>> On 14.07.15 at 17:04, <rcojocaru@bitdefender.com> wrote:
> On 07/14/2015 05:41 PM, Jan Beulich wrote:
>>>>> On 14.07.15 at 15:45, <rcojocaru@bitdefender.com> wrote:
>>> On 07/14/2015 03:35 PM, Jan Beulich wrote:
>>>>>>> On 13.07.15 at 19:14, <rcojocaru@bitdefender.com> wrote:
>>>>> --- a/xen/arch/x86/vm_event.c
>>>>> +++ b/xen/arch/x86/vm_event.c
>>>>> @@ -22,11 +22,19 @@
>>>>>  
>>>>>  #include <xen/sched.h>
>>>>>  #include <asm/hvm/hvm.h>
>>>>> +#include <asm/vm_event.h>
>>>>>  
>>>>>  int vm_event_init_domain(struct domain *d)
>>>>>  {
>>>>>      struct vcpu *v;
>>>>>  
>>>>> +    if ( !d->arch.event_write_data )
>>>>> +        d->arch.event_write_data = xzalloc_array(struct monitor_write_data,
>>>>> +                                                 d->max_vcpus);
>>>>
>>>> Looking at this again I wonder why the data isn't being made part of
>>>> struct arch_vcpu's vm_event sub-structure. That would also address
>>>> the complaint I have here about this not being a guaranteed maximum
>>>> page size runtime allocation.
>>>
>>> I think this is just how the initial suggestion was worded, I'll change it.
>> 
>> Right - after having sent the reply I started wondering whether
>> maybe I had asked for this. But if I did, then surely not with
>> xzalloc_array(), but vzalloc().
>> 
>> If you moved this into struct arch_vcpu (again), then its size would
>> likely call for the whole vm_event structure to become indirectly
>> accessed and allocated.
> 
> In that case would it suffice to just switch to vzalloc() in this case?

For now, yes.

> I'm not opposed to just placing all the data (this and the
> memory-content hiding data) in struct vm_event and allocate that as a
> whole, but that would change patch 1/3, 3/3 and also touch other code.

Would be a nice thing to clean up post-4.6.

Jan