[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 3/3] xen-hvm: Pass is_default to xen_hvm_init
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v6 3/3] xen-hvm: Pass is_default to xen_hvm_init |
Date: |
Tue, 17 Jun 2014 21:54:26 +0300 |
On Tue, Jun 17, 2014 at 02:32:10PM -0400, Don Slutz wrote:
> This is the xen part of "pc & q35: Add new machine opt max-ram-below-4g"
>
> This is done by adding a counter to know how many times
> pc_machine_set_max_ram_below_4g() is called. Therefore xen can know
> if a non-default setting is requested.
>
> Signed-off-by: Don Slutz <address@hidden>
> ---
> v6:
> Changed to work with the changes in #2:
> Added max_ram_below_4g_changed in order to know if it is a default value
> or a user specified one.
If you are special casing the default you might set it
to 0 just as well.
Sorry about not being clear, but what me and others were saying is that
we prefer an API should that there's no need for special-casing
user-specified values.
This approach isn't any better, it's just moving hacks around.
Of the two use-cases that you supplied, increasing RAM
below 4g and decreasing RAM below 4g, which do you
really care about?
If you really want a way to limit low RAM to 2g, then
how about this: set max_ram_below_4g_changed to 4g, and
apply the current limits *on top*.
So if user sets limit to 3.99 one still gets 3.5 or whatever.
But if you set it to 2G then you get 2G.
> Added "assert(pc_machine->max_ram_below_4g_changed > 0)" so that
> "make check" will abort on default not set for any of the pc or
> q35 machine types.
> Dropped "Acked-by: Stefano Stabellin" do to bigger change.
>
> hw/i386/pc.c | 1 +
> hw/i386/pc_piix.c | 2 ++
> hw/i386/pc_q35.c | 2 ++
> include/hw/i386/pc.h | 1 +
> include/hw/xen/xen.h | 2 +-
> xen-hvm-stub.c | 2 +-
> xen-hvm.c | 36 ++++++++++++++++++++----------------
> 7 files changed, 28 insertions(+), 18 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index a7f4f17..fc7e211 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1685,6 +1685,7 @@ static void pc_machine_set_max_ram_below_4g(Object
> *obj, Visitor *v,
> }
>
> pcms->max_ram_below_4g = value;
> + pcms->max_ram_below_4g_changed++;
> }
>
> static void pc_machine_initfn(Object *obj)
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 200c225..6072aec 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -129,7 +129,9 @@ static void pc_init1(MachineState *machine,
> below_4g_mem_size = machine->ram_size;
> }
>
> + assert(pc_machine->max_ram_below_4g_changed > 0);
> if (xen_enabled() && xen_hvm_init(&below_4g_mem_size, &above_4g_mem_size,
> + pc_machine->max_ram_below_4g_changed
> == 1,
> &ram_memory) != 0) {
> fprintf(stderr, "xen hardware virtual machine initialisation
> failed\n");
> exit(1);
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 43365cb..9386e44 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -118,7 +118,9 @@ static void pc_q35_init(MachineState *machine)
> below_4g_mem_size = machine->ram_size;
> }
>
> + assert(pc_machine->max_ram_below_4g_changed > 0);
> if (xen_enabled() && xen_hvm_init(&below_4g_mem_size, &above_4g_mem_size,
> + pc_machine->max_ram_below_4g_changed
> == 1,
> &ram_memory) != 0) {
> fprintf(stderr, "xen hardware virtual machine initialisation
> failed\n");
> exit(1);
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index f5672ee..34e488a 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -35,6 +35,7 @@ struct PCMachineState {
> HotplugHandler *acpi_dev;
>
> uint64_t max_ram_below_4g;
> + uint32_t max_ram_below_4g_changed;
> };
>
> #define PC_MACHINE_ACPI_DEVICE_PROP "acpi-device"
> diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
> index f71f2d8..6b94c14 100644
> --- a/include/hw/xen/xen.h
> +++ b/include/hw/xen/xen.h
> @@ -41,7 +41,7 @@ void xenstore_store_pv_console_info(int i, struct
> CharDriverState *chr);
>
> #if defined(NEED_CPU_H) && !defined(CONFIG_USER_ONLY)
> int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t
> *above_4g_mem_size,
> - MemoryRegion **ram_memory);
> + bool is_default, MemoryRegion **ram_memory);
> void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
> struct MemoryRegion *mr);
> void xen_modified_memory(ram_addr_t start, ram_addr_t length);
> diff --git a/xen-hvm-stub.c b/xen-hvm-stub.c
> index 2d98696..d1bdb76 100644
> --- a/xen-hvm-stub.c
> +++ b/xen-hvm-stub.c
> @@ -52,7 +52,7 @@ void xen_modified_memory(ram_addr_t start, ram_addr_t
> length)
> }
>
> int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t
> *above_4g_mem_size,
> - MemoryRegion **ram_memory)
> + bool is_default, MemoryRegion **ram_memory)
> {
> return 0;
> }
> diff --git a/xen-hvm.c b/xen-hvm.c
> index a0b6b5d..fba2c92 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -156,31 +156,34 @@ qemu_irq *xen_interrupt_controller_init(void)
> /* Memory Ops */
>
> static void xen_ram_init(ram_addr_t *below_4g_mem_size,
> - ram_addr_t *above_4g_mem_size,
> + ram_addr_t *above_4g_mem_size, bool is_default,
> ram_addr_t ram_size, MemoryRegion **ram_memory_p)
> {
> MemoryRegion *sysmem = get_system_memory();
> ram_addr_t block_len;
>
> - block_len = ram_size;
> - if (ram_size >= HVM_BELOW_4G_RAM_END) {
> - /* Xen does not allocate the memory continuously, and keep a hole at
> - * HVM_BELOW_4G_MMIO_START of HVM_BELOW_4G_MMIO_LENGTH
> + if (is_default) {
> + if (ram_size >= HVM_BELOW_4G_RAM_END) {
> + *above_4g_mem_size = ram_size - HVM_BELOW_4G_RAM_END;
> + *below_4g_mem_size = HVM_BELOW_4G_RAM_END;
> + } else {
> + *above_4g_mem_size = 0;
> + *below_4g_mem_size = ram_size;
> + }
> + }
> + if (!*above_4g_mem_size) {
> + block_len = ram_size;
> + } else {
> + /*
> + * Xen does not allocate the memory continuously, it keeps a
> + * hole of the size computed above or passed in.
> */
> - block_len += HVM_BELOW_4G_MMIO_LENGTH;
> + block_len = (1ULL << 32) + *above_4g_mem_size;
> }
> memory_region_init_ram(&ram_memory, NULL, "xen.ram", block_len);
> *ram_memory_p = &ram_memory;
> vmstate_register_ram_global(&ram_memory);
>
> - if (ram_size >= HVM_BELOW_4G_RAM_END) {
> - *above_4g_mem_size = ram_size - HVM_BELOW_4G_RAM_END;
> - *below_4g_mem_size = HVM_BELOW_4G_RAM_END;
> - } else {
> - *above_4g_mem_size = 0;
> - *below_4g_mem_size = ram_size;
> - }
> -
> memory_region_init_alias(&ram_640k, NULL, "xen.ram.640k",
> &ram_memory, 0, 0xa0000);
> memory_region_add_subregion(sysmem, 0, &ram_640k);
> @@ -962,7 +965,7 @@ static void xen_wakeup_notifier(Notifier *notifier, void
> *data)
> }
>
> int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t
> *above_4g_mem_size,
> - MemoryRegion **ram_memory)
> + bool is_default, MemoryRegion **ram_memory)
> {
> int i, rc;
> unsigned long ioreq_pfn;
> @@ -1040,7 +1043,8 @@ int xen_hvm_init(ram_addr_t *below_4g_mem_size,
> ram_addr_t *above_4g_mem_size,
>
> /* Init RAM management */
> xen_map_cache_init(xen_phys_offset_to_gaddr, state);
> - xen_ram_init(below_4g_mem_size, above_4g_mem_size, ram_size, ram_memory);
> + xen_ram_init(below_4g_mem_size, above_4g_mem_size, is_default, ram_size,
> + ram_memory);
>
> qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, state);
>
> --
> 1.8.4