| Message ID | 1334844527-18869-9-git-send-email-vasilis.liaskovitis@profitbricks.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Apr 19, 2012 at 04:08:46PM +0200, Vasilis Liaskovitis wrote: > Hotplugged memory is not persistent in the e820 memory maps. After hotplugging > a memslot and rebooting the VM, the hotplugged device is not present. > > A possible solution is to add an e820 for the new memslot in the acpi_piix4 > hot-add handler. On a reset, Seabios (see next patch in series) will enable all > memory devices for which it finds an e820 entry that covers the devices's address > range. > > On hot-remove, the acpi_piix4 handler will try to remove the e820 entry > corresponding to the device. This will work when no VM reboots happen > between hot-add and hot-remove, but it is not a sufficient solution in > general: Seabios and GuestOS merge adjacent e820 entries on machine reboot, > so the sequence hot-add/ rebootVM / hot-remove will fail to remove a > corresponding e820 entry at the hot-remove phase. > Why do you need this path and the next one? Bios can restore the state of memslots and build e820 map by reading mems_sts. > Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com> > --- > hw/acpi_piix4.c | 6 ++++++ > hw/pc.c | 28 ++++++++++++++++++++++++++++ > hw/pc.h | 1 + > 3 files changed, 35 insertions(+), 0 deletions(-) > > diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c > index 2921d18..2b5fd04 100644 > --- a/hw/acpi_piix4.c > +++ b/hw/acpi_piix4.c > @@ -619,6 +619,9 @@ static void piix4_memslot_eject(uint32_t addr, uint32_t val) > s = memslot_find_from_idx(start + idx); > assert(s != NULL); > memslot_depopulate(s); > + if (e820_del_entry(s->start, s->size, E820_RAM) == -EBUSY) > + PIIX4_DPRINTF("failed to remove e820 entry for memslot %u\n", > + s->idx); > } > val = val >> 1; > idx++; > @@ -634,6 +637,9 @@ static int piix4_memslot_hotplug(DeviceState *qdev, SysBusDevice *dev, int > > if (add) { > enable_mem_device(s, slot->idx); > + if (e820_add_entry(slot->start, slot->size, E820_RAM) == -EBUSY) > + PIIX4_DPRINTF("failed to add e820 entry for memslot %u\n", > + slot->idx); > } > else { > disable_mem_device(s, slot->idx); > diff --git a/hw/pc.c b/hw/pc.c > index f1f550a..04d243f 100644 > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -593,6 +593,34 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type) > return index; > } > > +int e820_del_entry(uint64_t address, uint64_t length, uint32_t type) > +{ > + int index = le32_to_cpu(e820_table.count); > + int search; > + struct e820_entry *entry; > + > + if (index == 0) > + return -EBUSY; > + search = index - 1; > + entry = &e820_table.entry[search]; > + while (search >= 0) { > + if ((entry->address == cpu_to_le64(address)) && > + (entry->length == cpu_to_le64(length)) && > + (entry->type == cpu_to_le32(type))){ > + if (search != index - 1) { > + memcpy(&e820_table.entry[search], &e820_table.entry[search + 1], > + sizeof(struct e820_entry) * (index - search)); > + } > + index--; > + e820_table.count = cpu_to_le32(index); > + return 1; > + } > + search--; > + entry = &e820_table.entry[search]; > + } > + return -EBUSY; > +} > + > static void bochs_bios_setup_hp_memslots(uint64_t *fw_cfg_slots); > > static void *bochs_bios_init(void) > diff --git a/hw/pc.h b/hw/pc.h > index 74d3369..4925e8c 100644 > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -226,5 +226,6 @@ void pc_system_firmware_init(MemoryRegion *rom_memory); > #define E820_UNUSABLE 5 > > int e820_add_entry(uint64_t, uint64_t, uint32_t); > +int e820_del_entry(uint64_t, uint64_t, uint32_t); > > #endif > -- > 1.7.9 -- Gleb.
On Sun, Apr 22, 2012 at 04:58:47PM +0300, Gleb Natapov wrote: > On Thu, Apr 19, 2012 at 04:08:46PM +0200, Vasilis Liaskovitis wrote: > > Hotplugged memory is not persistent in the e820 memory maps. After hotplugging > > a memslot and rebooting the VM, the hotplugged device is not present. > > > > A possible solution is to add an e820 for the new memslot in the acpi_piix4 > > hot-add handler. On a reset, Seabios (see next patch in series) will enable all > > memory devices for which it finds an e820 entry that covers the devices's address > > range. > > > > On hot-remove, the acpi_piix4 handler will try to remove the e820 entry > > corresponding to the device. This will work when no VM reboots happen > > between hot-add and hot-remove, but it is not a sufficient solution in > > general: Seabios and GuestOS merge adjacent e820 entries on machine reboot, > > so the sequence hot-add/ rebootVM / hot-remove will fail to remove a > > corresponding e820 entry at the hot-remove phase. > > > Why do you need this path and the next one? Bios can restore the state > of memslots and build e820 map by reading mems_sts. i see, that is a simpler solution. Since qemu currently creates most ram e820map entries and passes them to seabios, I tried to follow the same approach. But your suggestion makes things easier and we don't have to worry about merged e820 entries on hot-remove. I 'll rework it. thanks, Vasilis
On Mon, Apr 23, 2012 at 01:27:40PM +0200, Vasilis Liaskovitis wrote: > On Sun, Apr 22, 2012 at 04:58:47PM +0300, Gleb Natapov wrote: > > On Thu, Apr 19, 2012 at 04:08:46PM +0200, Vasilis Liaskovitis wrote: > > > Hotplugged memory is not persistent in the e820 memory maps. After hotplugging > > > a memslot and rebooting the VM, the hotplugged device is not present. > > > > > > A possible solution is to add an e820 for the new memslot in the acpi_piix4 > > > hot-add handler. On a reset, Seabios (see next patch in series) will enable all > > > memory devices for which it finds an e820 entry that covers the devices's address > > > range. > > > > > > On hot-remove, the acpi_piix4 handler will try to remove the e820 entry > > > corresponding to the device. This will work when no VM reboots happen > > > between hot-add and hot-remove, but it is not a sufficient solution in > > > general: Seabios and GuestOS merge adjacent e820 entries on machine reboot, > > > so the sequence hot-add/ rebootVM / hot-remove will fail to remove a > > > corresponding e820 entry at the hot-remove phase. > > > > > Why do you need this path and the next one? Bios can restore the state > > of memslots and build e820 map by reading mems_sts. > > i see, that is a simpler solution. Since qemu currently creates most ram e820map Quite the contrary. Qemu creates only one entry that Seabios can't figure by itself. > entries and passes them to seabios, I tried to follow the same approach. But > your suggestion makes things easier and we don't have to worry about merged e820 > entries on hot-remove. I 'll rework it. > thanks, > > Vasilis -- Gleb.
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c index 2921d18..2b5fd04 100644 --- a/hw/acpi_piix4.c +++ b/hw/acpi_piix4.c @@ -619,6 +619,9 @@ static void piix4_memslot_eject(uint32_t addr, uint32_t val) s = memslot_find_from_idx(start + idx); assert(s != NULL); memslot_depopulate(s); + if (e820_del_entry(s->start, s->size, E820_RAM) == -EBUSY) + PIIX4_DPRINTF("failed to remove e820 entry for memslot %u\n", + s->idx); } val = val >> 1; idx++; @@ -634,6 +637,9 @@ static int piix4_memslot_hotplug(DeviceState *qdev, SysBusDevice *dev, int if (add) { enable_mem_device(s, slot->idx); + if (e820_add_entry(slot->start, slot->size, E820_RAM) == -EBUSY) + PIIX4_DPRINTF("failed to add e820 entry for memslot %u\n", + slot->idx); } else { disable_mem_device(s, slot->idx); diff --git a/hw/pc.c b/hw/pc.c index f1f550a..04d243f 100644 --- a/hw/pc.c +++ b/hw/pc.c @@ -593,6 +593,34 @@ int e820_add_entry(uint64_t address, uint64_t length, uint32_t type) return index; } +int e820_del_entry(uint64_t address, uint64_t length, uint32_t type) +{ + int index = le32_to_cpu(e820_table.count); + int search; + struct e820_entry *entry; + + if (index == 0) + return -EBUSY; + search = index - 1; + entry = &e820_table.entry[search]; + while (search >= 0) { + if ((entry->address == cpu_to_le64(address)) && + (entry->length == cpu_to_le64(length)) && + (entry->type == cpu_to_le32(type))){ + if (search != index - 1) { + memcpy(&e820_table.entry[search], &e820_table.entry[search + 1], + sizeof(struct e820_entry) * (index - search)); + } + index--; + e820_table.count = cpu_to_le32(index); + return 1; + } + search--; + entry = &e820_table.entry[search]; + } + return -EBUSY; +} + static void bochs_bios_setup_hp_memslots(uint64_t *fw_cfg_slots); static void *bochs_bios_init(void) diff --git a/hw/pc.h b/hw/pc.h index 74d3369..4925e8c 100644 --- a/hw/pc.h +++ b/hw/pc.h @@ -226,5 +226,6 @@ void pc_system_firmware_init(MemoryRegion *rom_memory); #define E820_UNUSABLE 5 int e820_add_entry(uint64_t, uint64_t, uint32_t); +int e820_del_entry(uint64_t, uint64_t, uint32_t); #endif
Hotplugged memory is not persistent in the e820 memory maps. After hotplugging a memslot and rebooting the VM, the hotplugged device is not present. A possible solution is to add an e820 for the new memslot in the acpi_piix4 hot-add handler. On a reset, Seabios (see next patch in series) will enable all memory devices for which it finds an e820 entry that covers the devices's address range. On hot-remove, the acpi_piix4 handler will try to remove the e820 entry corresponding to the device. This will work when no VM reboots happen between hot-add and hot-remove, but it is not a sufficient solution in general: Seabios and GuestOS merge adjacent e820 entries on machine reboot, so the sequence hot-add/ rebootVM / hot-remove will fail to remove a corresponding e820 entry at the hot-remove phase. Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com> --- hw/acpi_piix4.c | 6 ++++++ hw/pc.c | 28 ++++++++++++++++++++++++++++ hw/pc.h | 1 + 3 files changed, 35 insertions(+), 0 deletions(-)