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

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

>>> On 16.07.15 at 10:48, <ravi.sahita@intel.com> wrote:
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>Sent: Tuesday, July 14, 2015 1:53 AM
>>>>> On 14.07.15 at 02:01, <ravi.sahita@intel.com> wrote:
>>>>From: Jan Beulich [mailto:JBeulich@suse.com]
>>>>Sent: Monday, July 13, 2015 1:01 AM
>>>>>>> On 10.07.15 at 23:48, <ravi.sahita@intel.com> wrote:
>>>>>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>>>>>Sent: Thursday, July 09, 2015 6:30 AM
>>>>>>>>> On 01.07.15 at 20:09, <edmund.h.white@intel.com> 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.

Jan