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

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 07/21/2015 11:13 AM, Jan Beulich wrote:
>>>> On 21.07.15 at 01:58, <edmund.h.white@xxxxxxxxx> wrote:
>> Add the basic data structures needed to support alternate p2m's and
>> the functions to initialise them and tear them down.
>>
>> Although Intel hardware can handle 512 EPTP's per hardware thread
>> concurrently, only 10 per domain are supported in this patch for
>> performance reasons.
>>
>> The iterator in hap_enable() does need to handle 512, so that is now
>> uint16_t.
> 
> Sigh - this one is still here (and the respective code unchanged).
> I'm not going to NAK the patch just because of this, but it really
> looks like you aren't trying to address comments even when
> they're trivial (and quick) to carry out and testing of the change
> comes as a side effect of you needing to test all the other changes
> as well.
> 
>> --- a/xen/arch/x86/hvm/Makefile
>> +++ b/xen/arch/x86/hvm/Makefile
>> @@ -1,6 +1,7 @@
>>  subdir-y += svm
>>  subdir-y += vmx
>>  
>> +obj-y += altp2m.o
> 
> Wasn't the outcome of the earlier discussion to put this in x86/mm,
> or possibly not even introduce a new file?

That was my recommendation [1] in response to v5, but there was no
response.  The mail seems to have been seen, however, since Andrew's
Reviewed-by was dropped.  (Perhaps they didn't notice the additional
comment further down.)

[1] marc.info/?i=<55A53159.4010703@xxxxxxxxxxxxx>

> Overall the situation didn't really change from v5 - the code from
> a pure functionality pov looks okay, but I don't see myself giving
> in on all the "minor" issues the patch introduces. If some were
> left adjusting of which really takes time to or risks breaking the
> code, I'd (reluctantly) give my ack, but not this way, I'm afraid.

This is a bit puzzling, and somewhat frustrating too -- to have
carefully combed through past versions, seen the comments and
discussion, and then carefully combed through this one to find that
nearly none of them have been addressed, even minor ones.

 -George


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