Re: [PATCH v3 0/4] Add max-ram-below-4g (was Add pci_hole_min_size machine option)

["Andreas Färber" <afaerber@suse.de>]

Am 25.03.2014 10:08, schrieb Michael S. Tsirkin:
> On Mon, Mar 24, 2014 at 07:55:32PM -0400, Don Slutz wrote:
>> Changes v2 to v3:
>>   Stefano Stabellini:
>>     Acked-by #1 "xen-all: Fix xen_hvm_init() to adjust pc memory"
>>     Adjust for code readability #4 "xen-all: Pass max_ram_below_4g to xen_hvm_init."
>>        Set max_ram_below_4g always and use it to calculate above_4g_mem_size,
>>        below_4g_mem_size.
>>
>> Changes v1 to v2:
>>   Michael S. Tsirkin:
>>     Rename option.
>>     Only add it to machine types that support it.
>>   Split into 4 parts.
>>
>> 1/4 -- xen-all: Fix xen_hvm_init() to adjust pc memory layout
>>
>>   This looks to be a possible bug that has yet to be found.
>>   below_4g_mem_size and above_4g_mem_size are stored in PcGuestInfo
>>   (pc_guest_info_init) which are currently not "correct".  This and
>>   4/4 change the same lines.
>>
>> 2/4 -- GlobalProperty: Display warning about unused -global
>>
>>     My testing showed that setting a global property on an object
>>     that is not used is not reported at all.  This is added to help
>>     when the new global is set but not used.  The negative not_used
>>     was picked so that all static objects are assumed to be used
>>     even when they are not.
>>
>> 3/4 -- pc & q35: Add new object pc-memory-layout
>>
>>   The objects that it might make sense to add this property to all
>>   get created too late.  So add a new object just to hold this
>>   property.  Name it so that it is expected that only pc (and q35)
>>   machine types support it.
>>
>> 4/4 -- xen-all: Pass max_ram_below_4g to xen_hvm_init
>>
>>   Seprate the xen only part of the change.  Currectly based on patch 1/4
>>
>> Don Slutz (4):
>>   xen-all: Fix xen_hvm_init() to adjust pc memory layout.
>>   GlobalProperty: Display warning about unused -global
>>   pc & q35: Add new object pc-memory-layout.
>>   xen-all: Pass max_ram_below_4g to xen_hvm_init.
>>
>>  hw/core/qdev-properties-system.c |  1 +
>>  hw/core/qdev-properties.c        | 15 ++++++++++++
>>  hw/i386/pc.c                     | 42 ++++++++++++++++++++++++++++++++
>>  hw/i386/pc_piix.c                | 52 +++++++++++++++++++++++++++-------------
>>  hw/i386/pc_q35.c                 | 50 ++++++++++++++++++++++++++------------
>>  include/hw/i386/pc.h             |  2 ++
>>  include/hw/qdev-core.h           |  1 +
>>  include/hw/qdev-properties.h     |  1 +
>>  include/hw/xen/xen.h             |  3 ++-
>>  vl.c                             |  2 ++
>>  xen-all.c                        | 46 +++++++++++++++++++----------------
>>  xen-stub.c                       |  3 ++-
>>  12 files changed, 165 insertions(+), 53 deletions(-)
> 
> Andreas, could you review pls, esp patch 2?

Michael, I don't see a dependency of the later patches on 2/4, so I'd
like to split that one out of this series and reiterate.

I'll have some minor comments on the rest.

Don, in general please do not end subjects with a dot.

CC'ing Eduardo and Igor who looked into 4G issues previously.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg