Re: [Xen-devel] [PATCH v3 05/13] x86/altp2m: basic data structures and support routines.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.07.15 at 00:39, <ravi.sahita@xxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>>Sent: Thursday, July 16, 2015 2:02 AM
>>
>>>>> On 16.07.15 at 10:48, <ravi.sahita@xxxxxxxxx> wrote:
>>>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>>>>Sent: Tuesday, July 14, 2015 1:53 AM
>>>>>>> On 14.07.15 at 02:01, <ravi.sahita@xxxxxxxxx> wrote:
>>>>>>From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>>>>>>Sent: Monday, July 13, 2015 1:01 AM
>>>>>>>>> On 10.07.15 at 23:48, <ravi.sahita@xxxxxxxxx> wrote:
>>>>>>>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>>>>>>>>Sent: Thursday, July 09, 2015 6:30 AM
>>>>>>>>>>> On 01.07.15 at 20:09, <edmund.h.white@xxxxxxxxx> wrote:
>>>>>>>>> @@ -294,6 +298,12 @@ struct arch_domain
>>>>>>>>>      struct p2m_domain *nested_p2m[MAX_NESTEDP2M];
>>>>>>>>>      mm_lock_t nested_p2m_lock;
>>>>>>>>>
>>>>>>>>> +    /* altp2m: allow multiple copies of host p2m */
>>>>>>>>> +    bool_t altp2m_active;
>>>>>>>>> +    struct p2m_domain *altp2m_p2m[MAX_ALTP2M];
>>>>>>>>> +    mm_lock_t altp2m_lock;
>>>>>>>>> +    uint64_t *altp2m_eptp;
>>>>>>>>
>>>>>>>>This is a non-insignificant increase of the structure size -
>>>>>>>>perhaps all of these should hang off of struct arch_domain via a
>>>>>>>>single, separately allocated pointer?
>>>>>>>
>>>>>>> Is this a nice-to-have - again we modelled after the nestedhvm
>>>>>>> extensions to the struct.
>>>>>>> This will affect a lot of our patch without really changing how
>>>>>>> much memory is allocated.
>>>>>>
>>>>>>I understand that. To a certain point I can agree to limit changes
>>>>>>to what is there at this stage. But you wanting to avoid addressing
>>>>>>concerns basically everywhere it's not a bug overextends this. Just
>>>>>>because the series was submitted late doesn't mean you should now
>>>>>>expect us to give in on any controversy regarding aspects we would
>>>>>>normally expect to be changed. This would basically encourage others
>>>>>>to submit their stuff late too in the
>>>>> future,
>>>>>>hoping for relaxed review.
>>>>>>
>>>>>
>>>>> Couple things - first, we have absorbed a lot of (good) feedback -
>>>>> thanks for that.
>>>>> Second, I don't think the series can be characterized as late
>>>>> (feedback from others welcome).
>>>>> V1 had almost the same structure and was submitted in January.
>>>>
>>>>Still we're at v3 only here, not v10 or beyond.
>>>>
>>>>> On this change - this will be a lot of effects on the code and we
>>>>> would like to avoid this one.
>>>>
>>>>While this may be a lot of mechanical change, I don't this presenting
>>>>any
>>> major
>>>>risk of breaking the code.
>>>
>>> On this one specific advice on how and where to implement such a
>>> change would be great just so that we don't thrash on this change.
>>
>>I don't follow - what to do here was said quite explicitly (still visible in 
> the
>>context above). I.e. I have no idea what additional advice you seek.
> 
> Ok that's fine - sorry if this was unclear - I was seeking if you had some 
> specific feedback on how to allocate and manage the dynamic altp2m struct etc 
> (if you had an opinion would be good to hear that).

xmalloc() / xzalloc(). What other alternatives would you see?

Jan


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