Re: [Xen-devel] [PATCH for 4.5 v7 1/1] Add mmio_hole_size

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.10.14 at 00:01, <dslutz@xxxxxxxxxxx> wrote:
> On 10/20/14 10:17, Ian Campbell wrote:
>> On Mon, 2014-10-13 at 08:51 -0400, Don Slutz wrote:
>>> --- a/tools/libxl/libxl_types.idl
>>> +++ b/tools/libxl/libxl_types.idl
>>> @@ -391,6 +391,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>>>                                          ("timeoffset",       string),
>>>                                          ("hpet",             
>>> libxl_defbool),
>>>                                          ("vpt_align",        
>>> libxl_defbool),
>>> +                                       ("mmio_hole_size",   uint64),
>> Please make this a MemKB at the libxl interface level and convert
>> internally to whatever hvmloader expects. It seems that a more
>> conventional name would also have a _memkb suffix. Perhaps
>> mmio_hole_memkb? (size seems to be implicit to me).
>>
> 
> Ok, Jan Beulich had proposed the name with the _size (on 27 Jun 2014), 
> but he
> did also say "(whether the xl config option should also get renamed I'm not
> sure - the xl maintainers will know )", (so I am taking this as ok from 
> Jan) I will
> rename to mmio_hole_memkb here.

My remark was really intended for you to actively inquire about the
config option name.

>>> +            /*
>>> +             * With HVM_BELOW_4G_RAM_END == 0xF0000000, mmio_hole_size
>>> +             * must be >= 256 MiB and <= 3840 MiB.
>> Isn't this just restating the if condition in a way which is liable to
>> get out of sync if the values of HVM_BELOW_4G_* ever changes?
> 
> Well, Konrad asked for this:
> [...]
> So I do not know which way to go here.

I have to admit that I think Konrad is asking for too much commentary
now and then. But in any event - Ian being one of the maintainers of
the affected code, what he asks you to do generally overrules what
any non-maintainer may have asked for.

Jan


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