[Qemu-devel] [PATCH 0/4] memory hotplug fixups when building with clang

[Qemu-devel] [PATCH 0/4] memory hotplug fixups when building with clang

[Igor Mammedov]

fixes build errors and warnings introduced by
memory hotplug series when building with clang

Igor Mammedov (4):
  pc: acpi: do not hardcode preprocessor
  fixup! pc: initialize memory hotplug address space
  fixup! pc: add memory hotplug handler to PC_MACHINE
  fixup! pc: implement pc-dimm device abstraction

 hw/i386/Makefile.objs    |    2 +-
 hw/i386/pc.c             |    6 +++---
 include/hw/mem/pc-dimm.h |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

[Qemu-devel] [PATCH 1/4] pc: acpi: do not hardcode preprocessor

[Igor Mammedov]

but use one provided by environment, in addition
force C style preprocessing so that 'gcc -E' or
"clang -E" wouldn't ignore .dsl files.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/Makefile.objs |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index a65473c..48014ab 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -18,7 +18,7 @@ iasl-option=$(shell if test -z "`$(1) $(2) 2>&1 > /dev/null`" \
 ifdef IASL
 #IASL Present. Generate hex files from .dsl
 hw/i386/%.hex: $(SRC_PATH)/hw/i386/%.dsl $(SRC_PATH)/scripts/acpi_extract_preprocess.py $(SRC_PATH)/scripts/acpi_extract.py
-	$(call quiet-command, cpp -P $(QEMU_DGFLAGS) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
+	$(call quiet-command, $(CPP) -x c -P $(QEMU_DGFLAGS) $(QEMU_INCLUDES) $< -o $*.dsl.i.orig, "  CPP $(TARGET_DIR)$*.dsl.i.orig")
 	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract_preprocess.py $*.dsl.i.orig > $*.dsl.i, "  ACPI_PREPROCESS $(TARGET_DIR)$*.dsl.i")
 	$(call quiet-command, $(IASL) $(call iasl-option,$(IASL),-Pn,) -vs -l -tc -p $* $*.dsl.i $(if $(V), , > /dev/null) 2>&1 ,"  IASL $(TARGET_DIR)$*.dsl.i")
 	$(call quiet-command, $(PYTHON) $(SRC_PATH)/scripts/acpi_extract.py $*.lst > $*.off, "  ACPI_EXTRACT $(TARGET_DIR)$*.off")
-- 
1.7.1

[Qemu-devel] [PATCH 2/4] fixup! pc: initialize memory hotplug address space

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 37344ce..73daa07 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1261,7 +1261,7 @@ FWCfgState *pc_memory_init(MachineState *machine,
 
         if ((pcms->hotplug_memory_base + hotplug_mem_size) <
             hotplug_mem_size) {
-            error_report("unsupported amount of maximum memory: %"PRIu64,
+            error_report("unsupported amount of maximum memory: " RAM_ADDR_FMT,
                          machine->maxram_size);
             exit(EXIT_FAILURE);
         }
-- 
1.7.1

[Qemu-devel] [PATCH 3/4] fixup! pc: add memory hotplug handler to PC_MACHINE

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/i386/pc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 73daa07..e993b0f 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1564,8 +1564,8 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
     PCDIMMDevice *dimm = PC_DIMM(dev);
     PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
     MemoryRegion *mr = ddc->get_memory_region(dimm);
-    ram_addr_t addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP,
-                                              &local_err);
+    uint64_t addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP,
+                                            &local_err);
     if (local_err) {
         goto out;
     }
-- 
1.7.1

Re: [Qemu-devel] [PATCH 3/4] fixup! pc: add memory hotplug handler to PC_MACHINE

[Michael S. Tsirkin]

On Mon, Jun 16, 2014 at 06:33:42PM +0200, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Note that properties are all int64.
I think we need a new kind of "RAM offset" property that
verifies that the supplied value is a valid
ram offset.

> ---
>  hw/i386/pc.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 73daa07..e993b0f 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1564,8 +1564,8 @@ static void pc_dimm_plug(HotplugHandler *hotplug_dev,
>      PCDIMMDevice *dimm = PC_DIMM(dev);
>      PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
>      MemoryRegion *mr = ddc->get_memory_region(dimm);
> -    ram_addr_t addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP,
> -                                              &local_err);
> +    uint64_t addr = object_property_get_int(OBJECT(dimm), PC_DIMM_ADDR_PROP,
> +                                            &local_err);
>      if (local_err) {
>          goto out;
>      }
> -- 
> 1.7.1

[Qemu-devel] [PATCH 4/4] fixup! pc: implement pc-dimm device abstraction

[Igor Mammedov]

fix typecheck error in DEFINE_PROP_UINT64 when building with with clang,
on OS X ram_addr_t is uintptr_t with size 8 bytes but compiler
still refuses to accept type_check() macro as valid.

It's still fixing consequences of how ram_addr_t is defined
but it's safer this way than to change ram_addr_t to uint64_t
unconditionally last minute.

TODO:
is to fix ram_addr_t on 32-bit host so that it could represent
64-bit guest addresses which is broken now if --disable-xen
configure option was used.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/mem/pc-dimm.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index 5f80d14..0f4a6ba 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -52,7 +52,7 @@ typedef struct PCDIMMDevice {
     DeviceState parent_obj;
 
     /* public */
-    ram_addr_t addr;
+    uint64_t addr;
     uint32_t node;
     int32_t slot;
     HostMemoryBackend *hostmem;
-- 
1.7.1

Re: [Qemu-devel] [PATCH 4/4] fixup! pc: implement pc-dimm device abstraction

[Michael S. Tsirkin]

On Mon, Jun 16, 2014 at 06:33:43PM +0200, Igor Mammedov wrote:
> fix typecheck error in DEFINE_PROP_UINT64 when building with with clang,
> on OS X ram_addr_t is uintptr_t with size 8 bytes but compiler
> still refuses to accept type_check() macro as valid.
> 
> It's still fixing consequences of how ram_addr_t is defined
> but it's safer this way than to change ram_addr_t to uint64_t
> unconditionally last minute.
> 
> TODO:
> is to fix ram_addr_t on 32-bit host so that it could represent
> 64-bit guest addresses which is broken now if --disable-xen
> configure option was used.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/mem/pc-dimm.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
> index 5f80d14..0f4a6ba 100644
> --- a/include/hw/mem/pc-dimm.h
> +++ b/include/hw/mem/pc-dimm.h
> @@ -52,7 +52,7 @@ typedef struct PCDIMMDevice {
>      DeviceState parent_obj;
>  
>      /* public */
> -    ram_addr_t addr;
> +    uint64_t addr;
>      uint32_t node;
>      int32_t slot;
>      HostMemoryBackend *hostmem;

This needs some thought.
HACKING explicitly says:
    Guest RAM memory offsets must use ram_addr_t, but only for RAM, it
    may not cover whole guest address space.

I think we need to validate addresses where they
are used/defined.

> -- 
> 1.7.1

Re: [Qemu-devel] [PATCH 0/4] memory hotplug fixups when building with clang

[Michael S. Tsirkin]

On Mon, Jun 16, 2014 at 06:33:39PM +0200, Igor Mammedov wrote:
> fixes build errors and warnings introduced by
> memory hotplug series when building with clang
> 
> Igor Mammedov (4):
>   pc: acpi: do not hardcode preprocessor
>   fixup! pc: initialize memory hotplug address space
>   fixup! pc: add memory hotplug handler to PC_MACHINE
>   fixup! pc: implement pc-dimm device abstraction

I applied patches 1-3.
I think patch 4 can be improved on, let's
merge it on top.

>  hw/i386/Makefile.objs    |    2 +-
>  hw/i386/pc.c             |    6 +++---
>  include/hw/mem/pc-dimm.h |    2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)